Rewrite vault core, issue AppRoles to minions (#62684)

* Rewrite vault core, orchestrate AppRoles for minions

This commit represents a fundamental rewrite in how Salt interacts with
Vault. The master should still be compatible with minions running the
old code. There should be no breaking changes to public interfaces and
the old configuration format should still apply.

Core:
- Issue AppRoles to minions
- Manage entities with templatable metadata for minions
- Use inbuilt Salt cache
- Separate config cache from token cache
- Cache: introduce connection-scope vs global scope

Utility module:
- Support being imported (__utils__ deprecation)
- Raise exceptions on queries to simplify response handling
- Add classes to wrap complexity, especially regarding KV v2
- Lay some groundwork for renewing tokens

Execution module:
- Add patch_secret
- Add version support to delete_secret
- Allow returning listed keys only in list_secret
- Add policy_[fetch/write/delete] and policies_list
- Add query for arbitrary API queries

State module:
- Make use of execution module
- Change output format

Docs:
- Update for new configuration format
- Correct examples
- Add configuration examples
- Add required policies

* Fix linting for rewritten vault integration

* Add pytest unit tests for utils.vault, fix found issues

* Fix old vault runner tests

* Rewrite vault sdb tests, migrate to pytests

* Adapt vault ext_pillar tests

* Adapt vault execution module tests, migrate to pytests

* Add more vault execution module unit tests

* Support python <3.7 (vault util), time-independent tests

* Add/migrate vault runner unit tests (pytest)

* Add vault state module pytests

* Fix tests lint

* Refactor Vault container fixture, move to session scope

* Fix for existing vault execution/sdb module integration tests

* Improve existing vault runner integration tests

* Fix vault test support, add list policies

* Add more functional execution module tests, fix deprecated warning

* Refactor vault pytest support

* Add integration tests, improve/fix caching/issue_params

* Improve caching behavior, fix tests

* Always use session cache as well
* Also flush session cache when requested
* Make KV metadata caching behavior configurable
* Update tests to account for changes from prev commit

* Allow to autodiscover platform default CA bundle

* Remove runner approle param overrides

There is no simple way to ensure they are kept.

* Add clear_cache runner function

* Also manage token metadata for issued secret IDs

* Cleanup tests

* Cleanup code, pylint logging suggestions

* Do not always invalidate config when verify=default

* Ensure concatted metadata lists are sorted

* Add changelog (partly)

* Work with legacy peer_run configuration as well

* Consume a token use regardless of status code

* Correct verify semantics

* Refine token uses handling, add changelog/tests for old issues

* Add changelog for main features

* Add test for issue 58580

* Fix vault docs

* Provide all old make_request functionality, add tests

* Allow token use override, add docstrings to query funcs

* Simplify config_location merge

* Cleanup

* Fix make_request warning

* Attempt to fix memory issues during CI test run

* Increase documented version

* Improve lease handling

* Refine lease ttl handling/add token lifecycle management

* Fix docs build

* Adapt formatting

* assert what you get against what you expect
* drop empty parentheses after wrapper
* use `is` to compare against strictly boolean vars

* Fix issue param overrides

* during pillar rendering, they were always reset by the master (for
  AppRoles)
* overrides were only respected for some settings (AppRoles)
* old config syntax was using the old syntax internally (tech debt)

* Introduce session-scoped cache

* Tokens with a single use left are unrenewable

* Allow override of flushing of cached leases during lookup

* Refactor cache classes, save lease data

* Rename session token cache key

* Add lease management utility

* Fix runner integration tests

after renaming the token cache key

* Do not overwrite data of cached leases after renewal

* Pass token_lifecycle to minions

* Do not fail syncing multiple approles/entities with pillar templates

* Ensure config cache expiration can be disabled

* Rename changelog files (.md)

* Declare vaultpolicylexer as parallel read safe

* Correct meta[data] payload key

For tokens it is `meta`, but for secret IDs, `metadata`.

* Reuse TCP connection

* Refactor utils module

* Ensure client is recreated after clearing cache

* Always use unwrap_client config as expected server

This should fix the test failure in the runner integration test
TestAppRoleIssuance::test_server_switch_does_not_break_minion_auth

* Ensure client is recreated after clearing cache 2

* Simulate patch for KV v1 or missing `patch` capability

* Add `patch` option to Vault SDB driver

* Reduce lease validity when revocation fails

* Extract AppRole/Identity API from runner into utils

* Revoke tokens, fire events, improve cache/exception handling

* Tokens (and therefore associated leases) are revoked when cleared by default
* It's possible to disable clearing cache when a perfectly valid token
  is available, but a PermissionDeniedError is encountered.
* UnwrapExceptions always cause an event to be fired
* It's possible to enable sending of events when
    a) cache is cleared
    b) a lease is requested from cache, but it is/will be invalid
* A VaultAuthException does not immediately lead to clearing
  the connection cache
* get_authd_client and others: multiple small enhancements and fixes

* Allow updating cached config w/o closing session

* Homogenize funcs, update docs, cleanup

* Minor internal fixes

`is_valid_for` is present on all lease-like objects, while `is_valid`
specifically should account for more, e.g. the number of uses.

The Vault API does not return 404 when a lookup fails.

* Add release note

* Address review remarks

* Fix release notes

* Remove loading minion_mods from factory

* Address other review remarks

* Add inline specification of trusted CA root cert

* Small QoL additions

* Fix lint

* Fix lint for Python >=3.8 support

* Add missing fixes

* Fix unit tests

In some cases, the `spec` calls were failing because the underlying
object was already patched

---------

Co-authored-by: Thomas Phipps <tphipps@vmware.com>

salt/pillar/vault.py

@@ -22,7 +22,7 @@ the ext_pillar section in the Salt master configuration.
       - vault: path=secret/salt
 
 Each key needs to have all the key-value pairs with the names you
-require. Avoid naming every key 'password' as you they will collide:
+require. Avoid naming every key 'password' as they will collide.
 
 If you want to nest results under a nesting_key name use the following format:
 
@@ -56,7 +56,7 @@ Multiple Vault sources may also be used:
       - 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.
+You can also use nesting here as well. Identical nesting keys will get merged.
 
 .. code-block:: yaml
 
@@ -131,6 +131,7 @@ This example takes the key value pairs returned from vault as follows:
 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 <ext_pillar_first>` master config setting.
+You cannot use pillar values sourced from Vault in pillar-templated policies.
 
 If a pillar pattern matches multiple paths, the results are merged according to
 the master configuration values :conf_master:`pillar_source_merging_strategy <pillar_source_merging_strategy>`
@@ -153,20 +154,14 @@ You can override the merging behavior per defined ext_pillar:
 
 import logging
 
-from requests.exceptions import HTTPError
-
 import salt.utils.dictupdate
+import salt.utils.vault as vault
+import salt.utils.vault.helpers as vhelpers
+from salt.exceptions import SaltException
 
 log = logging.getLogger(__name__)
 
 
-def __virtual__():
-    """
-    This module has no external dependencies
-    """
-    return True
-
-
 def ext_pillar(
     minion_id,  # pylint: disable=W0613
     pillar,  # pylint: disable=W0613
@@ -183,7 +178,6 @@ def ext_pillar(
     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=")]
@@ -195,30 +189,20 @@ def ext_pillar(
         "pillar_source_merging_strategy", "smart"
     )
     merge_lists = merge_lists or __opts__.get("pillar_merge_lists", False)
+
     vault_pillar = {}
 
     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_single = vault.read_kv(path, __opts__, __context__)
             vault_pillar = salt.utils.dictupdate.merge(
                 vault_pillar,
                 vault_pillar_single,
                 strategy=merge_strategy,
                 merge_lists=merge_lists,
             )
-        except HTTPError:
+        except SaltException:
             log.info("Vault secret not found for: %s", path)
 
     if nesting_key:
@@ -234,12 +218,10 @@ def _get_paths(path_pattern, minion_id, pillar):
 
     paths = []
     try:
-        for expanded_pattern in __utils__["vault.expand_pattern_lists"](
-            path_pattern, **mappings
-        ):
+        for expanded_pattern in vhelpers.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}")
+    log.debug("%s vault pillar paths: %s", minion_id, paths)
     return paths

salt/sdb/vault.py

@@ -9,7 +9,7 @@ Vault SDB Module
 
 This module allows access to Hashicorp Vault using an ``sdb://`` URI.
 
-Base configuration instructions are documented in the execution module docs.
+Base configuration instructions are documented in the :ref:`execution module docs <vault-setup>`.
 Below are noted extra configuration required for the sdb module, but the base
 configuration must also be completed.
 
@@ -37,12 +37,26 @@ The above URI is analogous to running the following vault command:
 .. code-block:: bash
 
     $ vault read -field=mypassword secret/passwords
+
+
+Further configuration
+---------------------
+The following options can be set in the profile:
+
+patch
+    When writing data, partially update the secret instead of overwriting it completely.
+    This is usually the expected behavior, since without this option,
+    each secret path can only contain a single mapping key safely.
+    Defaults to ``False`` for backwards-compatibility reasons.
+
+    .. versionadded:: 3007.0
 """
 
 
 import logging
 
 import salt.exceptions
+import salt.utils.vault as vault
 
 log = logging.getLogger(__name__)
 
@@ -58,62 +72,50 @@ def set_(key, value, profile=None):
     else:
         path, key = key.rsplit("/", 1)
     data = {key: value}
+    curr_data = {}
+    profile = profile or {}
 
-    version2 = __utils__["vault.is_v2"](path)
-    if version2["v2"]:
-        path = version2["data"]
-        data = {"data": data}
+    if profile.get("patch"):
+        try:
+            # Patching only works on existing secrets.
+            # Save the current data if patching is enabled
+            # to write it back later, if any errors happen in patch_kv.
+            # This also checks that the path exists, otherwise patching fails as well.
+            curr_data = vault.read_kv(path, __opts__, __context__)
+            vault.patch_kv(path, data, __opts__, __context__)
+            return True
+        except (vault.VaultNotFoundError, vault.VaultPermissionDeniedError):
+            pass
 
+    curr_data.update(data)
     try:
-        url = "v1/{}".format(path)
-        response = __utils__["vault.make_request"]("POST", url, json=data)
-
-        if response.status_code != 204:
-            response.raise_for_status()
+        vault.write_kv(path, data, __opts__, __context__)
         return True
-    except Exception as e:  # pylint: disable=broad-except
-        log.error("Failed to write secret! %s: %s", type(e).__name__, e)
-        raise salt.exceptions.CommandExecutionError(e)
+    except Exception as err:  # pylint: disable=broad-except
+        log.error("Failed to write secret! %s: %s", type(err).__name__, err)
+        raise salt.exceptions.CommandExecutionError(err) from err
 
 
 def get(key, profile=None):
     """
     Get a value from the vault service
     """
+    full_path = key
     if "?" in key:
         path, key = key.split("?")
     else:
         path, key = key.rsplit("/", 1)
 
-    version2 = __utils__["vault.is_v2"](path)
-    if version2["v2"]:
-        path = version2["data"]
-
     try:
-        url = "v1/{}".format(path)
-        response = __utils__["vault.make_request"]("GET", url)
-        if response.status_code == 404:
-            if version2["v2"]:
-                path = version2["data"] + "/" + key
-                url = "v1/{}".format(path)
-                response = __utils__["vault.make_request"]("GET", url)
-                if response.status_code == 404:
-                    return None
-            else:
-                return None
-        if response.status_code != 200:
-            response.raise_for_status()
-        data = response.json()["data"]
-
-        if version2["v2"]:
-            if key in data["data"]:
-                return data["data"][key]
-            else:
-                return data["data"]
-        else:
-            if key in data:
-                return data[key]
+        try:
+            res = vault.read_kv(path, __opts__, __context__)
+            if key in res:
+                return res[key]
+            return None
+        except vault.VaultNotFoundError:
+            return vault.read_kv(full_path, __opts__, __context__)
+    except vault.VaultNotFoundError:
         return None
-    except Exception as e:  # pylint: disable=broad-except
-        log.error("Failed to read secret! %s: %s", type(e).__name__, e)
-        raise salt.exceptions.CommandExecutionError(e)
+    except Exception as err:  # pylint: disable=broad-except
+        log.error("Failed to read secret! %s: %s", type(err).__name__, err)
+        raise salt.exceptions.CommandExecutionError(err) from err

salt/states/vault.py

@@ -1,6 +1,7 @@
 """
 States for managing Hashicorp Vault.
-Currently handles policies. Configuration instructions are documented in the execution module docs.
+Currently handles policies.
+Configuration instructions are documented in the :ref:`execution module docs <vault-setup>`.
 
 :maintainer:    SaltStack
 :maturity:      new
@@ -13,12 +14,14 @@ Currently handles policies. Configuration instructions are documented in the exe
 import difflib
 import logging
 
+from salt.exceptions import CommandExecutionError
+
 log = logging.getLogger(__name__)
 
 __deprecated__ = (
     3009,
     "vault",
-    "https://github.com/saltstack/saltext-vault",
+    "https://github.com/salt-extensions/saltext-vault",
 )
 
 
@@ -47,85 +50,88 @@ def policy_present(name, rules):
                 }
 
     """
-    url = f"v1/sys/policy/{name}"
-    response = __utils__["vault.make_request"]("GET", url)
+    ret = {"name": name, "changes": {}, "result": True, "comment": ""}
+
     try:
-        if response.status_code == 200:
-            return _handle_existing_policy(name, rules, response.json()["rules"])
-        elif response.status_code == 404:
-            return _create_new_policy(name, rules)
-        else:
-            response.raise_for_status()
-    except Exception as e:  # pylint: disable=broad-except
-        return {
-            "name": name,
-            "changes": {},
-            "result": False,
-            "comment": f"Failed to get policy: {e}",
-        }
+        existing_rules = __salt__["vault.policy_fetch"](name)
+    except CommandExecutionError as err:
+        ret["result"] = False
+        ret["comment"] = f"Failed to read policy: {err}"
+        return ret
 
-
-def _create_new_policy(name, rules):
-    if __opts__["test"]:
-        return {
-            "name": name,
-            "changes": {name: {"old": "", "new": rules}},
-            "result": None,
-            "comment": "Policy would be created",
-        }
-
-    payload = {"rules": rules}
-    url = f"v1/sys/policy/{name}"
-    response = __utils__["vault.make_request"]("PUT", url, json=payload)
-    if response.status_code not in [200, 204]:
-        return {
-            "name": name,
-            "changes": {},
-            "result": False,
-            "comment": f"Failed to create policy: {response.reason}",
-        }
-
-    return {
-        "name": name,
-        "result": True,
-        "changes": {name: {"old": None, "new": rules}},
-        "comment": "Policy was created",
-    }
-
-
-def _handle_existing_policy(name, new_rules, existing_rules):
-    ret = {"name": name}
-    if new_rules == existing_rules:
-        ret["result"] = True
-        ret["changes"] = {}
+    if existing_rules == rules:
         ret["comment"] = "Policy exists, and has the correct content"
         return ret
 
-    change = "".join(
+    diff = "".join(
         difflib.unified_diff(
-            existing_rules.splitlines(True), new_rules.splitlines(True)
+            (existing_rules or "").splitlines(True), rules.splitlines(True)
         )
     )
+
+    ret["changes"] = {name: diff}
+
     if __opts__["test"]:
         ret["result"] = None
-        ret["changes"] = {name: {"change": change}}
-        ret["comment"] = "Policy would be changed"
+        ret["comment"] = "Policy would be " + (
+            "created" if existing_rules is None else "updated"
+        )
         return ret
 
-    payload = {"rules": new_rules}
-
-    url = f"v1/sys/policy/{name}"
-    response = __utils__["vault.make_request"]("PUT", url, json=payload)
-    if response.status_code not in [200, 204]:
+    try:
+        __salt__["vault.policy_write"](name, rules)
+        ret["comment"] = "Policy has been " + (
+            "created" if existing_rules is None else "updated"
+        )
+        return ret
+    except CommandExecutionError as err:
         return {
             "name": name,
             "changes": {},
             "result": False,
-            "comment": f"Failed to change policy: {response.reason}",
+            "comment": f"Failed to write policy: {err}",
         }
 
-    ret["result"] = True
-    ret["changes"] = {name: {"change": change}}
-    ret["comment"] = "Policy was updated"
 
-    return ret
+def policy_absent(name):
+    """
+    Ensure a Vault policy with the given name and rules is absent.
+
+    name
+        The name of the policy
+    """
+    ret = {"name": name, "changes": {}, "result": True, "comment": ""}
+
+    try:
+        existing_rules = __salt__["vault.policy_fetch"](name)
+    except CommandExecutionError as err:
+        ret["result"] = False
+        ret["comment"] = f"Failed to read policy: {err}"
+        return ret
+
+    if existing_rules is None:
+        ret["comment"] = "Policy is already absent"
+        return ret
+
+    ret["changes"] = {"deleted": name}
+
+    if __opts__["test"]:
+        ret["result"] = None
+        ret["comment"] = "Policy would be deleted"
+        return ret
+
+    try:
+        if not __salt__["vault.policy_delete"](name):
+            raise CommandExecutionError(
+                "Policy was initially reported as existent, but seemed to be "
+                "absent while deleting."
+            )
+        ret["comment"] = "Policy has been deleted"
+        return ret
+    except CommandExecutionError as err:
+        return {
+            "name": name,
+            "changes": {},
+            "result": False,
+            "comment": f"Failed to delete policy: {err}",
+        }

tests/pytests/functional/modules/test_vault.py

@@ -1,19 +1,24 @@
-import json
 import logging
-import time
 
 import pytest
 
-import salt.utils.path
-from tests.support.runtests import RUNTIME_VARS
+# pylint: disable=unused-import
+from tests.support.pytest.vault import (
+    vault_container_version,
+    vault_delete_policy,
+    vault_delete_secret,
+    vault_environ,
+    vault_list_policies,
+    vault_list_secrets,
+    vault_read_policy,
+    vault_write_policy,
+)
 
 pytestmark = [
     pytest.mark.slow_test,
     pytest.mark.skip_if_binaries_missing("dockerd", "vault", "getent"),
 ]
 
-VAULT_BINARY = salt.utils.path.which("vault")
-
 log = logging.getLogger(__name__)
 
 
@@ -21,123 +26,35 @@ log = logging.getLogger(__name__)
 def minion_config_overrides(vault_port):
     return {
         "vault": {
-            "url": "http://127.0.0.1:{}".format(vault_port),
             "auth": {
                 "method": "token",
                 "token": "testsecret",
-                "uses": 0,
-                "policies": [
-                    "testpolicy",
-                ],
+            },
+            "server": {
+                "url": f"http://127.0.0.1:{vault_port}",
             },
         }
     }
 
 
-def vault_container_version_id(value):
-    return "vault=={}".format(value)
-
-
-@pytest.fixture(
-    scope="module",
-    params=["0.9.6", "1.3.1", "latest"],
-    ids=vault_container_version_id,
-)
-def vault_container_version(request, salt_factories, vault_port, shell):
-    vault_version = request.param
-    config = {
-        "backend": {"file": {"path": "/vault/file"}},
-        "default_lease_ttl": "168h",
-        "max_lease_ttl": "720h",
-        "disable_mlock": False,
-    }
-
-    factory = salt_factories.get_container(
-        "vault",
-        "ghcr.io/saltstack/salt-ci-containers/vault:{}".format(vault_version),
-        check_ports=[vault_port],
-        container_run_kwargs={
-            "ports": {"8200/tcp": vault_port},
-            "environment": {
-                "VAULT_DEV_ROOT_TOKEN_ID": "testsecret",
-                "VAULT_LOCAL_CONFIG": json.dumps(config),
-            },
-            "cap_add": "IPC_LOCK",
-        },
-        pull_before_start=True,
-        skip_on_pull_failure=True,
-        skip_if_docker_client_not_connectable=True,
-    )
-    with factory.started() as factory:
-        attempts = 0
-        while attempts < 3:
-            attempts += 1
-            time.sleep(1)
-            ret = shell.run(
-                VAULT_BINARY,
-                "login",
-                "token=testsecret",
-                env={"VAULT_ADDR": "http://127.0.0.1:{}".format(vault_port)},
-            )
-            if ret.returncode == 0:
-                break
-            log.debug("Failed to authenticate against vault:\n%s", ret)
-            time.sleep(4)
-        else:
-            pytest.fail("Failed to login to vault")
-
-        ret = shell.run(
-            VAULT_BINARY,
-            "policy",
-            "write",
-            "testpolicy",
-            "{}/vault.hcl".format(RUNTIME_VARS.FILES),
-            env={"VAULT_ADDR": "http://127.0.0.1:{}".format(vault_port)},
-        )
-        if ret.returncode != 0:
-            log.debug("Failed to assign policy to vault:\n%s", ret)
-            pytest.fail("unable to assign policy to vault")
-        yield vault_version
-
-
 @pytest.fixture(scope="module")
 def sys_mod(modules):
     return modules.sys
 
 
 @pytest.fixture
-def vault(loaders, modules, vault_container_version, shell, vault_port):
+def vault(loaders, modules, vault_container_version):
     try:
         yield modules.vault
     finally:
         # We're explicitly using the vault CLI and not the salt vault module
         secret_path = "secret/my"
-        ret = shell.run(
-            VAULT_BINARY,
-            "kv",
-            "list",
-            "--format=json",
-            secret_path,
-            env={"VAULT_ADDR": "http://127.0.0.1:{}".format(vault_port)},
-        )
-        if ret.returncode == 0:
-            for secret in ret.data:
-                secret_path = "secret/my/{}".format(secret)
-                ret = shell.run(
-                    VAULT_BINARY,
-                    "kv",
-                    "delete",
-                    secret_path,
-                    env={"VAULT_ADDR": "http://127.0.0.1:{}".format(vault_port)},
-                )
-                ret = shell.run(
-                    VAULT_BINARY,
-                    "kv",
-                    "metadata",
-                    "delete",
-                    secret_path,
-                    env={"VAULT_ADDR": "http://127.0.0.1:{}".format(vault_port)},
-                )
+        for secret in vault_list_secrets(secret_path):
+            vault_delete_secret(f"{secret_path}/{secret}", metadata=True)
+        policies = vault_list_policies()
+        for policy in ["functional_test_policy", "policy_write_test"]:
+            if policy in policies:
+                vault_delete_policy(policy)
 
 
 @pytest.mark.windows_whitelisted
@@ -253,12 +170,36 @@ def existing_secret(vault, vault_container_version):
         assert ret == expected_write
 
 
+@pytest.fixture
+def existing_secret_version(existing_secret, vault, vault_container_version):
+    ret = vault.write_secret("secret/my/secret", user="foo", password="hunter1")
+    assert ret
+    assert ret["version"] == 2
+    ret = vault.read_secret("secret/my/secret")
+    assert ret
+    assert ret["password"] == "hunter1"
+
+
 @pytest.mark.usefixtures("existing_secret")
 def test_delete_secret(vault):
     ret = vault.delete_secret("secret/my/secret")
     assert ret is True
 
 
+@pytest.mark.usefixtures("existing_secret_version")
+@pytest.mark.parametrize("vault_container_version", ["1.3.1", "latest"], indirect=True)
+def test_delete_secret_versions(vault, vault_container_version):
+    ret = vault.delete_secret("secret/my/secret", 1)
+    assert ret is True
+    ret = vault.read_secret("secret/my/secret")
+    assert ret
+    assert ret["password"] == "hunter1"
+    ret = vault.delete_secret("secret/my/secret", 2)
+    assert ret is True
+    ret = vault.read_secret("secret/my/secret", default="__was_deleted__")
+    assert ret == "__was_deleted__"
+
+
 @pytest.mark.usefixtures("existing_secret")
 def test_list_secrets(vault):
     ret = vault.list_secrets("secret/my/")
@@ -268,8 +209,66 @@ def test_list_secrets(vault):
 
 
 @pytest.mark.usefixtures("existing_secret")
+@pytest.mark.parametrize("vault_container_version", ["1.3.1", "latest"], indirect=True)
 def test_destroy_secret_kv2(vault, vault_container_version):
-    if vault_container_version == "0.9.6":
-        pytest.skip("Test not applicable to vault=={}".format(vault_container_version))
     ret = vault.destroy_secret("secret/my/secret", "1")
     assert ret is True
+
+
+@pytest.mark.usefixtures("existing_secret")
+@pytest.mark.parametrize("vault_container_version", ["latest"], indirect=True)
+def test_patch_secret(vault, vault_container_version):
+    ret = vault.patch_secret("secret/my/secret", password="baz")
+    assert ret
+    expected_write = {"destroyed": False, "deletion_time": ""}
+    for key in list(ret):
+        if key not in expected_write:
+            ret.pop(key)
+    assert ret == expected_write
+    ret = vault.read_secret("secret/my/secret")
+    assert ret == {"user": "foo", "password": "baz"}
+
+
+@pytest.fixture
+def policy_rules():
+    return """\
+path "secret/some/thing" {
+    capabilities = ["read"]
+}
+    """
+
+
+@pytest.fixture
+def existing_policy(policy_rules, vault_container_version):
+    vault_write_policy("functional_test_policy", policy_rules)
+    try:
+        yield
+    finally:
+        vault_delete_policy("functional_test_policy")
+
+
+@pytest.mark.usefixtures("existing_policy")
+def test_policy_fetch(vault, policy_rules):
+    ret = vault.policy_fetch("functional_test_policy")
+    assert ret == policy_rules
+    ret = vault.policy_fetch("__does_not_exist__")
+    assert ret is None
+
+
+def test_policy_write(vault, policy_rules):
+    ret = vault.policy_write("policy_write_test", policy_rules)
+    assert ret is True
+    assert vault_read_policy("policy_write_test") == policy_rules
+
+
+@pytest.mark.usefixtures("existing_policy")
+def test_policy_delete(vault):
+    ret = vault.policy_delete("functional_test_policy")
+    assert ret is True
+    assert "functional_test_policy" not in vault_list_policies()
+
+
+@pytest.mark.usefixtures("existing_policy")
+def test_policies_list(vault):
+    ret = vault.policies_list()
+    assert "functional_test_policy" in ret

tests/pytests/functional/utils/test_vault.py

@@ -0,0 +1,164 @@
+import logging
+
+import pytest
+import requests.exceptions
+
+# pylint: disable=unused-import
+from tests.support.pytest.vault import (
+    vault_container_version,
+    vault_delete_secret,
+    vault_environ,
+    vault_list_secrets,
+    vault_read_secret,
+    vault_write_secret,
+)
+
+pytestmark = [
+    pytest.mark.slow_test,
+    pytest.mark.skip_if_binaries_missing("dockerd", "vault", "getent"),
+]
+
+log = logging.getLogger(__name__)
+
+
+@pytest.fixture(scope="module")
+def minion_config_overrides(vault_port):
+    return {
+        "vault": {
+            "auth": {
+                "method": "token",
+                "token": "testsecret",
+            },
+            "server": {
+                "url": f"http://127.0.0.1:{vault_port}",
+            },
+        }
+    }
+
+
+@pytest.fixture
+def vault(loaders):
+    return loaders.utils.vault
+
+
+@pytest.fixture(scope="module", autouse=True)
+def vault_testing_data(vault_container_version):
+    vault_write_secret("secret/utils/read", success="yup")
+    vault_write_secret("secret/utils/deleteme", success="nope")
+    try:
+        yield
+    finally:
+        secret_path = "secret/utils"
+        for secret in vault_list_secrets(secret_path):
+            vault_delete_secret(f"{secret_path}/{secret}", metadata=True)
+
+
+def test_make_request_get_unauthd(vault):
+    """
+    Test that unauthenticated GET requests are possible
+    """
+    res = vault.make_request("GET", "/v1/sys/health")
+    assert res.status_code == 200
+    assert res.json()
+    assert "initialized" in res.json()
+
+
+def test_make_request_get_authd(vault, vault_container_version):
+    """
+    Test that authenticated GET requests are possible
+    """
+    endpoint = "secret/utils/read"
+    if vault_container_version in ["1.3.1", "latest"]:
+        endpoint = "secret/data/utils/read"
+
+    res = vault.make_request("GET", f"/v1/{endpoint}")
+    assert res.status_code == 200
+    data = res.json()["data"]
+    if vault_container_version in ["1.3.1", "latest"]:
+        data = data["data"]
+    assert "success" in data
+    assert data["success"] == "yup"
+
+
+def test_make_request_post_json(vault, vault_container_version):
+    """
+    Test that POST requests are possible with json param
+    """
+    data = {"success": "yup"}
+    endpoint = "secret/utils/write"
+
+    if vault_container_version in ["1.3.1", "latest"]:
+        data = {"data": data}
+        endpoint = "secret/data/utils/write"
+    res = vault.make_request("POST", f"/v1/{endpoint}", json=data)
+    assert res.status_code in [200, 204]
+    assert vault_read_secret("secret/utils/write") == {"success": "yup"}
+
+
+def test_make_request_post_data(vault, vault_container_version):
+    """
+    Test that POST requests are possible with data param
+    """
+    data = '{"success": "yup_data"}'
+    endpoint = "secret/utils/write"
+
+    if vault_container_version in ["1.3.1", "latest"]:
+        data = '{"data": {"success": "yup_data"}}'
+        endpoint = "secret/data/utils/write"
+    res = vault.make_request("POST", f"/v1/{endpoint}", data=data)
+    assert res.status_code in [200, 204]
+    assert vault_read_secret("secret/utils/write") == {"success": "yup_data"}
+
+
+def test_make_request_delete(vault, vault_container_version):
+    """
+    Test that DELETE requests are possible
+    """
+    endpoint = "secret/utils/deleteme"
+    if vault_container_version in ["1.3.1", "latest"]:
+        endpoint = "secret/data/utils/deleteme"
+
+    res = vault.make_request("DELETE", f"/v1/{endpoint}")
+    assert res.status_code in [200, 204]
+    assert vault_read_secret("secret/utils/deleteme") is None
+
+
+def test_make_request_list(vault, vault_container_version):
+    """
+    Test that LIST requests are possible
+    """
+    endpoint = "secret/utils"
+    if vault_container_version in ["1.3.1", "latest"]:
+        endpoint = "secret/metadata/utils"
+
+    res = vault.make_request("LIST", f"/v1/{endpoint}")
+    assert res.status_code == 200
+    assert res.json()["data"]["keys"] == vault_list_secrets("secret/utils")
+
+
+def test_make_request_token_override(vault, vault_container_version):
+    """
+    Test that overriding the token in use is possible
+    """
+    endpoint = "secret/utils/read"
+    if vault_container_version in ["1.3.1", "latest"]:
+        endpoint = "secret/data/utils/read"
+
+    res = vault.make_request("GET", f"/v1/{endpoint}", token="invalid")
+    assert res.status_code == 403
+
+
+def test_make_request_url_override(vault, vault_container_version):
+    """
+    Test that overriding the server URL is possible
+    """
+    endpoint = "secret/utils/read"
+    if vault_container_version in ["1.3.1", "latest"]:
+        endpoint = "secret/data/utils/read"
+
+    with pytest.raises(
+        requests.exceptions.ConnectionError, match=".*Max retries exceeded with url:.*"
+    ):
+        vault.make_request(
+            "GET", f"/v1/{endpoint}", vault_url="http://127.0.0.1:1", timeout=2
+        )

tests/pytests/integration/modules/test_vault.py

@@ -0,0 +1,375 @@
+"""
+Tests for the Vault module
+"""
+
+import logging
+import shutil
+import time
+
+import pytest
+from saltfactories.utils import random_string
+
+# pylint: disable=unused-import
+from tests.support.pytest.vault import (
+    vault_container_version,
+    vault_delete_secret,
+    vault_environ,
+    vault_list_secrets,
+    vault_write_secret,
+)
+
+log = logging.getLogger(__name__)
+
+
+pytestmark = [
+    pytest.mark.slow_test,
+    pytest.mark.skip_if_binaries_missing("dockerd", "vault", "getent"),
+    pytest.mark.usefixtures("vault_container_version"),
+]
+
+
+@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 vault_salt_master(
+    salt_factories, pillar_state_tree, vault_port, vault_master_config
+):
+    factory = salt_factories.salt_master_daemon(
+        "vault-exemaster", defaults=vault_master_config
+    )
+    with factory.started():
+        yield factory
+
+
+@pytest.fixture(scope="class")
+def vault_salt_minion(vault_salt_master, vault_minion_config):
+    assert vault_salt_master.is_running()
+    factory = vault_salt_master.salt_minion_daemon(
+        random_string("vault-exeminion", uppercase=False),
+        defaults=vault_minion_config,
+    )
+    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 vault_minion_config():
+    return {"open_mode": True, "grains": {}}
+
+
+@pytest.fixture(scope="class")
+def vault_salt_run_cli(vault_salt_master):
+    return vault_salt_master.salt_run_cli()
+
+
+@pytest.fixture(scope="class")
+def vault_salt_call_cli(vault_salt_minion):
+    return vault_salt_minion.salt_call_cli()
+
+
+@pytest.fixture(scope="class")
+def pillar_dual_use_tree(
+    vault_salt_master,
+    vault_salt_minion,
+):
+    top_pillar_contents = f"""
+    base:
+      '{vault_salt_minion.id}':
+        - testvault
+    """
+    test_pillar_contents = """
+    test:
+      foo: bar
+      jvmdump_pubkey: {{ salt["vault.read_secret"]("secret/test/jvmdump/ssh_key", "public_key") }}
+      jenkins_pubkey: {{ salt["vault.read_secret"]("secret/test/jenkins/master/ssh_key", "public_key") }}
+    """
+    top_file = vault_salt_master.pillar_tree.base.temp_file(
+        "top.sls", top_pillar_contents
+    )
+    test_file = vault_salt_master.pillar_tree.base.temp_file(
+        "testvault.sls", test_pillar_contents
+    )
+
+    with top_file, test_file:
+        yield
+
+
+@pytest.fixture(scope="class")
+def vault_testing_data(vault_container_version):
+    vault_write_secret("secret/test/jvmdump/ssh_key", public_key="yup_dump")
+    vault_write_secret("secret/test/jenkins/master/ssh_key", public_key="yup_master")
+    vault_write_secret("secret/test/deleteme", pls=":)")
+    try:
+        yield
+    finally:
+        vault_delete_secret("secret/test/jvmdump/ssh_key")
+        vault_delete_secret("secret/test/jenkins/master/ssh_key")
+        for x in ["deleteme", "write"]:
+            if x in vault_list_secrets("secret/test"):
+                vault_delete_secret(f"secret/test/{x}")
+
+
+@pytest.mark.usefixtures("vault_testing_data", "pillar_dual_use_tree")
+@pytest.mark.parametrize("vault_container_version", ["1.3.1", "latest"], indirect=True)
+class TestSingleUseToken:
+    """
+    Single-use tokens and read operations on versions below 0.10.0
+    do not work since the necessary metadata lookup consumes a use
+    there without caching metadata information (sys/internal/mounts/:path
+    is not available, hence not an unauthenticated endpoint).
+    It is impossible to differentiate between the endpoint not being
+    available and the token not having the correct permissions.
+    """
+
+    @pytest.fixture(scope="class")
+    def vault_master_config(self, pillar_state_tree, vault_port):
+        return {
+            "pillar_roots": {"base": [str(pillar_state_tree)]},
+            "open_mode": True,
+            "peer_run": {
+                ".*": [
+                    "vault.get_config",
+                    "vault.generate_new_token",
+                ],
+            },
+            "vault": {
+                "auth": {"token": "testsecret"},
+                "cache": {
+                    "backend": "file",
+                },
+                "issue": {
+                    "type": "token",
+                    "token": {
+                        "params": {
+                            "num_uses": 1,
+                        }
+                    },
+                },
+                "policies": {
+                    "assign": [
+                        "salt_minion",
+                    ]
+                },
+                "server": {
+                    "url": f"http://127.0.0.1:{vault_port}",
+                },
+            },
+            "minion_data_cache": False,
+        }
+
+    def test_vault_read_secret(self, vault_salt_call_cli):
+        """
+        Test that the Vault module can fetch a single secret when tokens
+        are issued with uses=1.
+        """
+        ret = vault_salt_call_cli.run(
+            "vault.read_secret", "secret/test/jvmdump/ssh_key"
+        )
+        assert ret.returncode == 0
+        assert ret.data == {"public_key": "yup_dump"}
+
+    def test_vault_read_secret_can_fetch_more_than_one_secret_in_one_run(
+        self,
+        vault_salt_call_cli,
+        vault_salt_minion,
+        caplog,
+    ):
+        """
+        Test that the Vault module can fetch multiple secrets during
+        a single run when tokens are issued with uses=1.
+        Issue #57561
+        """
+        ret = vault_salt_call_cli.run("saltutil.refresh_pillar", wait=True)
+        assert ret.returncode == 0
+        assert ret.data is True
+        ret = vault_salt_call_cli.run("pillar.items")
+        assert ret.returncode == 0
+        assert ret.data
+        assert "Pillar render error" not in caplog.text
+        assert "test" in ret.data
+        assert "jvmdump_pubkey" in ret.data["test"]
+        assert ret.data["test"]["jvmdump_pubkey"] == "yup_dump"
+        assert "jenkins_pubkey" in ret.data["test"]
+        assert ret.data["test"]["jenkins_pubkey"] == "yup_master"
+
+    def test_vault_write_secret(self, vault_salt_call_cli):
+        """
+        Test that the Vault module can write a single secret when tokens
+        are issued with uses=1.
+        """
+        ret = vault_salt_call_cli.run(
+            "vault.write_secret", "secret/test/write", success="yup"
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert "write" in vault_list_secrets("secret/test")
+
+    def test_vault_delete_secret(self, vault_salt_call_cli):
+        """
+        Test that the Vault module can delete a single secret when tokens
+        are issued with uses=1.
+        """
+        ret = vault_salt_call_cli.run("vault.delete_secret", "secret/test/deleteme")
+        assert ret.returncode == 0
+        assert ret.data
+        assert "delete" not in vault_list_secrets("secret/test")
+
+
+class TestTokenMinimumTTLUnrenewable:
+    """
+    Test that a new token is requested when the current one does not
+    fulfill minimum_ttl and cannot be renewed
+    """
+
+    @pytest.fixture(scope="class")
+    def vault_master_config(self, vault_port):
+        return {
+            "pillar_roots": {},
+            "open_mode": True,
+            "peer_run": {
+                ".*": [
+                    "vault.get_config",
+                    "vault.generate_new_token",
+                ],
+            },
+            "vault": {
+                "auth": {"token": "testsecret"},
+                "cache": {
+                    "backend": "file",
+                },
+                "issue": {
+                    "type": "token",
+                    "token": {
+                        "params": {
+                            "num_uses": 0,
+                            "explicit_max_ttl": 180,
+                        }
+                    },
+                },
+                "policies": {
+                    "assign": [
+                        "salt_minion",
+                    ]
+                },
+                "server": {
+                    "url": f"http://127.0.0.1:{vault_port}",
+                },
+            },
+            "minion_data_cache": False,
+        }
+
+    @pytest.fixture(scope="class")
+    def vault_minion_config(self):
+        return {
+            "open_mode": True,
+            "grains": {},
+            "vault": {
+                "auth": {
+                    "token_lifecycle": {"minimum_ttl": 178, "renew_increment": None}
+                }
+            },
+        }
+
+    def test_minimum_ttl_is_respected(self, vault_salt_call_cli):
+        # create token by looking it up
+        ret = vault_salt_call_cli.run("vault.query", "GET", "auth/token/lookup-self")
+        assert ret.data
+        assert ret.returncode == 0
+        # wait
+        time_before = time.time()
+        while time.time() - time_before < 3:
+            time.sleep(0.1)
+        # reissue token by looking it up
+        ret_new = vault_salt_call_cli.run(
+            "vault.query", "GET", "auth/token/lookup-self"
+        )
+        assert ret_new.returncode == 0
+        assert ret_new.data
+        # ensure a new token was created, even though the previous one would have been
+        # valid still
+        assert ret_new.data["data"]["id"] != ret.data["data"]["id"]
+
+
+class TestTokenMinimumTTLRenewable:
+    """
+    Test that tokens are renewed and minimum_ttl is respected
+    """
+
+    @pytest.fixture(scope="class")
+    def vault_master_config(self, vault_port):
+        return {
+            "pillar_roots": {},
+            "open_mode": True,
+            "peer_run": {
+                ".*": [
+                    "vault.get_config",
+                    "vault.generate_new_token",
+                ],
+            },
+            "vault": {
+                "auth": {"token": "testsecret"},
+                "cache": {
+                    "backend": "file",
+                },
+                "issue": {
+                    "type": "token",
+                    "token": {
+                        "params": {
+                            "num_uses": 0,
+                            "ttl": 180,
+                        }
+                    },
+                },
+                "policies": {
+                    "assign": [
+                        "salt_minion",
+                    ]
+                },
+                "server": {
+                    "url": f"http://127.0.0.1:{vault_port}",
+                },
+            },
+            "minion_data_cache": False,
+        }
+
+    @pytest.fixture(scope="class")
+    def vault_minion_config(self):
+        return {
+            "open_mode": True,
+            "grains": {},
+            "vault": {
+                "auth": {
+                    "token_lifecycle": {"minimum_ttl": 177, "renew_increment": None}
+                }
+            },
+        }
+
+    def test_minimum_ttl_is_respected(self, vault_salt_call_cli):
+        # create token by looking it up
+        ret = vault_salt_call_cli.run("vault.query", "GET", "auth/token/lookup-self")
+        assert ret.data
+        assert ret.returncode == 0
+        # wait
+        time_before = time.time()
+        while time.time() - time_before < 4:
+            time.sleep(0.1)
+        # renew token by looking it up
+        ret_new = vault_salt_call_cli.run(
+            "vault.query", "GET", "auth/token/lookup-self"
+        )
+        assert ret_new.returncode == 0
+        assert ret_new.data
+        # ensure the current token's validity has been extended
+        assert ret_new.data["data"]["id"] == ret.data["data"]["id"]
+        assert ret_new.data["data"]["expire_time"] > ret.data["data"]["expire_time"]

tests/pytests/integration/sdb/conftest.py

@@ -11,7 +11,6 @@ def pillar_tree(salt_master, salt_minion):
         salt_minion.id
     )
     sdb_pillar_file = """
-    test_vault_pillar_sdb: sdb://sdbvault/secret/test/test_pillar_sdb/foo
     test_etcd_pillar_sdb: sdb://sdbetcd/secret/test/test_pillar_sdb/foo
     """
     top_tempfile = salt_master.pillar_tree.base.temp_file("top.sls", top_file)

tests/pytests/integration/sdb/test_vault.py

@@ -1,17 +1,19 @@
 """
 Integration tests for the vault modules
 """
-import json
 import logging
-import subprocess
-import time
 
 import pytest
-from pytestshellutils.utils.processes import ProcessResult
+from saltfactories.utils import random_string
 
-import salt.utils.path
-from tests.support.helpers import PatchedEnviron
-from tests.support.runtests import RUNTIME_VARS
+# pylint: disable=unused-import
+from tests.support.pytest.vault import (
+    vault_container_version,
+    vault_delete_secret,
+    vault_environ,
+    vault_list_secrets,
+    vault_write_secret,
+)
 
 log = logging.getLogger(__name__)
 
@@ -19,237 +21,143 @@ log = logging.getLogger(__name__)
 pytestmark = [
     pytest.mark.slow_test,
     pytest.mark.skip_if_binaries_missing("dockerd", "vault", "getent"),
+    pytest.mark.usefixtures("vault_container_version"),
 ]
 
 
-@pytest.fixture(scope="module")
-def patched_environ(vault_port):
-    with PatchedEnviron(VAULT_ADDR="http://127.0.0.1:{}".format(vault_port)):
+@pytest.fixture(scope="class")
+def pillar_tree(vault_salt_master, vault_salt_minion):
+    top_file = f"""
+    base:
+      '{vault_salt_minion.id}':
+        - sdb
+    """
+    sdb_pillar_file = """
+    test_vault_pillar_sdb: sdb://sdbvault/secret/test/test_pillar_sdb/foo
+    """
+    top_tempfile = vault_salt_master.pillar_tree.base.temp_file("top.sls", top_file)
+    sdb_tempfile = vault_salt_master.pillar_tree.base.temp_file(
+        "sdb.sls", sdb_pillar_file
+    )
+
+    with top_tempfile, sdb_tempfile:
         yield
 
 
-def vault_container_version_id(value):
-    return "vault=={}".format(value)
-
-
-@pytest.fixture(
-    scope="module",
-    autouse=True,
-    params=["0.9.6", "1.3.1", "latest"],
-    ids=vault_container_version_id,
-)
-def vault_container_version(request, salt_factories, vault_port, patched_environ):
-    vault_version = request.param
-    vault_binary = salt.utils.path.which("vault")
-    config = {
-        "backend": {"file": {"path": "/vault/file"}},
-        "default_lease_ttl": "168h",
-        "max_lease_ttl": "720h",
-    }
-    factory = salt_factories.get_container(
-        "vault",
-        "ghcr.io/saltstack/salt-ci-containers/vault:{}".format(vault_version),
-        check_ports=[vault_port],
-        container_run_kwargs={
-            "ports": {"8200/tcp": vault_port},
-            "environment": {
-                "VAULT_DEV_ROOT_TOKEN_ID": "testsecret",
-                "VAULT_LOCAL_CONFIG": json.dumps(config),
-            },
-            "cap_add": "IPC_LOCK",
-        },
-        pull_before_start=True,
-        skip_on_pull_failure=True,
-        skip_if_docker_client_not_connectable=True,
-    )
-    with factory.started() as factory:
-        attempts = 0
-        while attempts < 3:
-            attempts += 1
-            time.sleep(1)
-            proc = subprocess.run(
-                [vault_binary, "login", "token=testsecret"],
-                check=False,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE,
-                universal_newlines=True,
-            )
-            if proc.returncode == 0:
-                break
-            ret = ProcessResult(
-                returncode=proc.returncode,
-                stdout=proc.stdout,
-                stderr=proc.stderr,
-                cmdline=proc.args,
-            )
-            log.debug("Failed to authenticate against vault:\n%s", ret)
-            time.sleep(4)
-        else:
-            pytest.fail("Failed to login to vault")
-
-        proc = subprocess.run(
-            [
-                vault_binary,
-                "policy",
-                "write",
-                "testpolicy",
-                "{}/vault.hcl".format(RUNTIME_VARS.FILES),
+@pytest.fixture(scope="class")
+def vault_master_config(vault_port):
+    return {
+        "open_mode": True,
+        "peer_run": {
+            ".*": [
+                "vault.get_config",
+                "vault.generate_new_token",
             ],
-            check=False,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE,
-            universal_newlines=True,
-        )
-        if proc.returncode != 0:
-            ret = ProcessResult(
-                returncode=proc.returncode,
-                stdout=proc.stdout,
-                stderr=proc.stderr,
-                cmdline=proc.args,
-            )
-            log.debug("Failed to assign policy to vault:\n%s", ret)
-            pytest.fail("unable to assign policy to vault")
-        if vault_version in ("1.3.1", "latest"):
-            proc = subprocess.run(
-                [vault_binary, "secrets", "enable", "kv-v2"],
-                check=False,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE,
-                universal_newlines=True,
-            )
-            ret = ProcessResult(
-                returncode=proc.returncode,
-                stdout=proc.stdout,
-                stderr=proc.stderr,
-                cmdline=proc.args,
-            )
-            if proc.returncode != 0:
-                log.debug("Failed to enable kv-v2:\n%s", ret)
-                pytest.fail("Could not enable kv-v2")
-
-            if "path is already in use at kv-v2/" in proc.stdout:
-                pass
-            elif "Success" in proc.stdout:
-                pass
-            else:
-                log.debug("Failed to enable kv-v2:\n%s", ret)
-                pytest.fail("Could not enable kv-v2 {}".format(proc.stdout))
-            if vault_version == "latest":
-                proc = subprocess.run(
-                    [
-                        vault_binary,
-                        "secrets",
-                        "enable",
-                        "-version=2",
-                        "-path=salt/",
-                        "kv",
-                    ],
-                    check=False,
-                    stdout=subprocess.PIPE,
-                    stderr=subprocess.PIPE,
-                    universal_newlines=True,
-                )
-                ret = ProcessResult(
-                    returncode=proc.returncode,
-                    stdout=proc.stdout,
-                    stderr=proc.stderr,
-                    cmdline=proc.args,
-                )
-                if proc.returncode != 0:
-                    log.debug("Failed to enable kv-v2:\n%s", ret)
-                    pytest.fail("Could not enable kv-v2")
-
-                if "path is already in use at kv-v2/" in proc.stdout:
-                    pass
-                elif "Success" in proc.stdout:
-                    proc = subprocess.run(
-                        [
-                            vault_binary,
-                            "kv",
-                            "put",
-                            "salt/user1",
-                            "password=p4ssw0rd",
-                            "desc=test user",
-                        ],
-                        check=False,
-                        stdout=subprocess.PIPE,
-                        stderr=subprocess.PIPE,
-                        universal_newlines=True,
-                    )
-                    ret = ProcessResult(
-                        returncode=proc.returncode,
-                        stdout=proc.stdout,
-                        stderr=proc.stderr,
-                        cmdline=proc.args,
-                    )
-                    if proc.returncode != 0:
-                        log.debug("Failed to enable kv-v2:\n%s", ret)
-                        pytest.fail("Could not enable kv-v2")
-                    if "path is already in use at kv-v2/" in proc.stdout:
-                        pass
-                    elif "created_time" in proc.stdout:
-                        proc = subprocess.run(
-                            [
-                                vault_binary,
-                                "kv",
-                                "put",
-                                "salt/user/user1",
-                                "password=p4ssw0rd",
-                                "desc=test user",
-                            ],
-                            check=False,
-                            stdout=subprocess.PIPE,
-                            stderr=subprocess.PIPE,
-                            universal_newlines=True,
-                        )
-                        ret = ProcessResult(
-                            returncode=proc.returncode,
-                            stdout=proc.stdout,
-                            stderr=proc.stderr,
-                            cmdline=proc.args,
-                        )
-                        if proc.returncode != 0:
-                            log.debug("Failed to enable kv-v2:\n%s", ret)
-                            pytest.fail("Could not enable kv-v2")
-
-                        if "path is already in use at kv-v2/" in proc.stdout:
-                            pass
-                        elif "created_time" in proc.stdout:
-                            proc = subprocess.run(
-                                [vault_binary, "kv", "get", "salt/user1"],
-                                check=False,
-                                stdout=subprocess.PIPE,
-                                stderr=subprocess.PIPE,
-                                universal_newlines=True,
-                            )
-                            ret = ProcessResult(
-                                returncode=proc.returncode,
-                                stdout=proc.stdout,
-                                stderr=proc.stderr,
-                                cmdline=proc.args,
-                            )
-
-                else:
-                    log.debug("Failed to enable kv-v2:\n%s", ret)
-                    pytest.fail("Could not enable kv-v2 {}".format(proc.stdout))
-        yield vault_version
+        },
+        "vault": {
+            "auth": {
+                "token": "testsecret",
+            },
+            "issue": {
+                "token": {
+                    "params": {
+                        "num_uses": 0,
+                    }
+                }
+            },
+            "policies": {
+                "assign": [
+                    "salt_minion",
+                ]
+            },
+            "server": {
+                "url": f"http://127.0.0.1:{vault_port}",
+            },
+        },
+        "minion_data_cache": True,
+    }
 
 
-def test_sdb(salt_call_cli):
+@pytest.fixture(scope="class")
+def vault_salt_master(salt_factories, vault_port, vault_master_config):
+    factory = salt_factories.salt_master_daemon(
+        "vault-sdbmaster", defaults=vault_master_config
+    )
+    with factory.started():
+        yield factory
+
+
+@pytest.fixture(scope="class")
+def sdb_profile():
+    return {}
+
+
+@pytest.fixture(scope="class")
+def vault_salt_minion(vault_salt_master, sdb_profile):
+    assert vault_salt_master.is_running()
+    config = {"open_mode": True, "grains": {}, "sdbvault": {"driver": "vault"}}
+    config["sdbvault"].update(sdb_profile)
+    factory = vault_salt_master.salt_minion_daemon(
+        random_string("vault-sdbminion", uppercase=False),
+        defaults=config,
+    )
+    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 vault_salt_call_cli(vault_salt_minion):
+    return vault_salt_minion.salt_call_cli()
+
+
+@pytest.fixture(scope="class")
+def vault_salt_run_cli(vault_salt_master):
+    return vault_salt_master.salt_run_cli()
+
+
+@pytest.fixture
+def kv_root_dual_item(vault_container_version):
+    if vault_container_version == "latest":
+        vault_write_secret("salt/user1", password="p4ssw0rd", desc="test user")
+        vault_write_secret("salt/user/user1", password="p4ssw0rd", desc="test user")
+    yield
+    if vault_container_version == "latest":
+        vault_delete_secret("salt/user1")
+        vault_delete_secret("salt/user/user1")
+
+
+@pytest.mark.parametrize("vault_container_version", ["1.3.1", "latest"], indirect=True)
+def test_sdb_kv_kvv2_path_local(salt_call_cli, vault_container_version):
     ret = salt_call_cli.run(
-        "sdb.set", uri="sdb://sdbvault/secret/test/test_sdb/foo", value="bar"
+        "--local",
+        "sdb.set",
+        uri="sdb://sdbvault/kv-v2/test/test_sdb_local/foo",
+        value="local",
     )
     assert ret.returncode == 0
     assert ret.data is True
-    ret = salt_call_cli.run("sdb.get", uri="sdb://sdbvault/secret/test/test_sdb/foo")
-    assert ret.returncode == 0
+    ret = salt_call_cli.run(
+        "--local", "sdb.get", "sdb://sdbvault/kv-v2/test/test_sdb_local/foo"
+    )
     assert ret.data
-    assert ret.data == "bar"
+    assert ret.data == "local"
+
+
+@pytest.mark.usefixtures("kv_root_dual_item")
+@pytest.mark.parametrize("vault_container_version", ["latest"], indirect=True)
+def test_sdb_kv_dual_item(salt_call_cli, vault_container_version):
+    ret = salt_call_cli.run("--local", "sdb.get", "sdb://sdbvault/salt/data/user1")
+    assert ret.data
+    assert ret.data == {"desc": "test user", "password": "p4ssw0rd"}
 
 
 def test_sdb_runner(salt_run_cli):
     ret = salt_run_cli.run(
-        "sdb.set", uri="sdb://sdbvault/secret/test/test_sdb_runner/foo", value="bar"
+        "sdb.set", uri="sdb://sdbvault/secret/test/test_sdb_runner/foo", value="runner"
     )
     assert ret.returncode == 0
     assert ret.data is True
@@ -258,40 +166,146 @@ def test_sdb_runner(salt_run_cli):
     )
     assert ret.returncode == 0
     assert ret.stdout
-    assert ret.stdout == "bar"
+    assert ret.stdout == "runner"
 
 
-def test_config(salt_call_cli, pillar_tree):
-    ret = salt_call_cli.run(
-        "sdb.set", uri="sdb://sdbvault/secret/test/test_pillar_sdb/foo", value="bar"
+@pytest.mark.usefixtures("pillar_tree")
+class TestSDB:
+    def test_sdb(self, vault_salt_call_cli):
+        ret = vault_salt_call_cli.run(
+            "sdb.set", uri="sdb://sdbvault/secret/test/test_sdb/foo", value="bar"
+        )
+        assert ret.returncode == 0
+        assert ret.data is True
+        ret = vault_salt_call_cli.run(
+            "sdb.get", uri="sdb://sdbvault/secret/test/test_sdb/foo"
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert ret.data == "bar"
+
+    def test_config(self, vault_salt_call_cli):
+        ret = vault_salt_call_cli.run(
+            "sdb.set", uri="sdb://sdbvault/secret/test/test_pillar_sdb/foo", value="baz"
+        )
+        assert ret.returncode == 0
+        assert ret.data is True
+        ret = vault_salt_call_cli.run("config.get", "test_vault_pillar_sdb")
+        assert ret.returncode == 0
+        assert ret.data
+        assert ret.data == "baz"
+
+
+class TestGetOrSetHashSingleUseToken:
+    @pytest.fixture(scope="class")
+    def vault_master_config(self, vault_port):
+        return {
+            "open_mode": True,
+            "peer_run": {
+                ".*": [
+                    "vault.get_config",
+                    "vault.generate_new_token",
+                ],
+            },
+            "vault": {
+                "auth": {"token": "testsecret"},
+                "cache": {
+                    "backend": "file",
+                },
+                "issue": {
+                    "type": "token",
+                    "token": {
+                        "params": {
+                            "num_uses": 1,
+                        }
+                    },
+                },
+                "policies": {
+                    "assign": [
+                        "salt_minion",
+                    ],
+                },
+                "server": {
+                    "url": f"http://127.0.0.1:{vault_port}",
+                },
+            },
+            "minion_data_cache": True,
+        }
+
+    @pytest.fixture
+    def get_or_set_absent(self):
+        secret_path = "secret/test"
+        secret_name = "sdb_get_or_set_hash"
+        ret = vault_list_secrets(secret_path)
+        if secret_name in ret:
+            vault_delete_secret(f"{secret_path}/{secret_name}")
+        ret = vault_list_secrets(secret_path)
+        assert secret_name not in ret
+        try:
+            yield
+        finally:
+            ret = vault_list_secrets(secret_path)
+            if secret_name in ret:
+                vault_delete_secret(f"{secret_path}/{secret_name}")
+
+    @pytest.mark.usefixtures("get_or_set_absent")
+    @pytest.mark.parametrize(
+        "vault_container_version", ["1.3.1", "latest"], indirect=True
     )
-    assert ret.returncode == 0
-    assert ret.data is True
-    ret = salt_call_cli.run("config.get", "test_vault_pillar_sdb")
-    assert ret.returncode == 0
-    assert ret.data
-    assert ret.data == "bar"
+    def test_sdb_get_or_set_hash_single_use_token(self, vault_salt_call_cli):
+        """
+        Test that sdb.get_or_set_hash works with uses=1.
+        This fails for versions that do not have the sys/internal/ui/mounts/:path
+        endpoint (<0.10.0) because the path metadata lookup consumes a token use there.
+        Issue #60779
+        """
+        ret = vault_salt_call_cli.run(
+            "sdb.get_or_set_hash",
+            "sdb://sdbvault/secret/test/sdb_get_or_set_hash/foo",
+            10,
+        )
+        assert ret.returncode == 0
+        result = ret.data
+        assert result
+        ret = vault_salt_call_cli.run(
+            "sdb.get_or_set_hash",
+            "sdb://sdbvault/secret/test/sdb_get_or_set_hash/foo",
+            10,
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert ret.data == result
 
 
-def test_sdb_kv2_kvv2_path_local(salt_call_cli, vault_container_version):
-    if vault_container_version not in ["1.3.1", "latest"]:
-        pytest.skip("Test not applicable to vault {}".format(vault_container_version))
+class TestSDBSetPatch:
+    @pytest.fixture(scope="class")
+    def sdb_profile(self):
+        return {"patch": True}
 
-    ret = salt_call_cli.run(
-        "sdb.set", uri="sdb://sdbvault/kv-v2/test/test_sdb/foo", value="bar"
-    )
-    assert ret.returncode == 0
-    assert ret.data is True
-    ret = salt_call_cli.run(
-        "--local", "sdb.get", "sdb://sdbvault/kv-v2/test/test_sdb/foo"
-    )
-    assert ret.data
-    assert ret.data == "bar"
-
-
-def test_sdb_kv_dual_item(salt_call_cli, vault_container_version):
-    if vault_container_version not in ["latest"]:
-        pytest.skip("Test not applicable to vault {}".format(vault_container_version))
-    ret = salt_call_cli.run("--local", "sdb.get", "sdb://sdbvault/salt/data/user1")
-    assert ret.data
-    assert ret.data == {"desc": "test user", "password": "p4ssw0rd"}
+    def test_sdb_set(self, vault_salt_call_cli):
+        # Write to an empty path
+        ret = vault_salt_call_cli.run(
+            "sdb.set", uri="sdb://sdbvault/secret/test/test_sdb_patch/foo", value="bar"
+        )
+        assert ret.returncode == 0
+        assert ret.data is True
+        # Write to an existing path, this should not overwrite the previous key
+        ret = vault_salt_call_cli.run(
+            "sdb.set", uri="sdb://sdbvault/secret/test/test_sdb_patch/bar", value="baz"
+        )
+        assert ret.returncode == 0
+        assert ret.data is True
+        # Ensure the first value is still there
+        ret = vault_salt_call_cli.run(
+            "sdb.get", uri="sdb://sdbvault/secret/test/test_sdb_patch/foo"
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert ret.data == "bar"
+        # Ensure the second value was written
+        ret = vault_salt_call_cli.run(
+            "sdb.get", uri="sdb://sdbvault/secret/test/test_sdb_patch/bar"
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert ret.data == "baz"

tests/pytests/unit/modules/test_vault.py

@@ -1,161 +1,441 @@
-"""
-Test case for the vault execution module
-"""
-
+import logging
 
 import pytest
 
+import salt.exceptions
 import salt.modules.vault as vault
-from salt.exceptions import CommandExecutionError
-from tests.support.mock import MagicMock, patch
+import salt.utils.vault as vaultutil
+from tests.support.mock import ANY, patch
 
 
 @pytest.fixture
 def configure_loader_modules():
     return {
         vault: {
-            "__grains__": {"id": "foo"},
-            "__utils__": {
-                "vault.is_v2": MagicMock(
-                    return_value={
-                        "v2": True,
-                        "data": "secrets/data/mysecret",
-                        "metadata": "secrets/metadata/mysecret",
-                        "type": "kv",
-                    }
-                ),
-            },
-        },
+            "__grains__": {"id": "test-minion"},
+        }
     }
 
 
 @pytest.fixture
-def path():
-    return "foo/bar/"
+def data():
+    return {"foo": "bar"}
 
 
-def test_read_secret_v1():
-    """
-    Test salt.modules.vault.read_secret function
-    """
-    version = {"v2": False, "data": None, "metadata": None, "type": None}
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    mock_vault.return_value.json.return_value = {"data": {"key": "test"}}
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        vault_return = vault.read_secret("/secret/my/secret")
-
-    assert vault_return == {"key": "test"}
-
-
-def test_read_secret_v1_key():
-    """
-    Test salt.modules.vault.read_secret function specifying key
-    """
-    version = {"v2": False, "data": None, "metadata": None, "type": None}
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    mock_vault.return_value.json.return_value = {"data": {"key": "somevalue"}}
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        vault_return = vault.read_secret("/secret/my/secret", "key")
-
-    assert vault_return == "somevalue"
-
-
-def test_read_secret_v2():
-    """
-    Test salt.modules.vault.read_secret function for v2 of kv secret backend
-    """
-    # given path secrets/mysecret generate v2 output
-    version = {
-        "v2": True,
-        "data": "secrets/data/mysecret",
-        "metadata": "secrets/metadata/mysecret",
-        "type": "kv",
-    }
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    v2_return = {
-        "data": {
-            "data": {"akey": "avalue"},
-            "metadata": {
-                "created_time": "2018-10-23T20:21:55.042755098Z",
-                "destroyed": False,
-                "version": 13,
-                "deletion_time": "",
-            },
-        }
+@pytest.fixture
+def policy_response():
+    return {
+        "name": "test-policy",
+        "rules": 'path "secret/*"\\n{\\n  capabilities = ["read"]\\n}',
     }
 
-    mock_vault.return_value.json.return_value = v2_return
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        # Validate metadata returned
-        vault_return = vault.read_secret("/secret/my/secret", metadata=True)
-        assert "data" in vault_return
-        assert "metadata" in vault_return
-        # Validate just data returned
-        vault_return = vault.read_secret("/secret/my/secret")
-        assert "akey" in vault_return
 
-
-def test_read_secret_v2_key():
-    """
-    Test salt.modules.vault.read_secret function for v2 of kv secret backend
-    with specified key
-    """
-    # given path secrets/mysecret generate v2 output
-    version = {
-        "v2": True,
-        "data": "secrets/data/mysecret",
-        "metadata": "secrets/metadata/mysecret",
-        "type": "kv",
-    }
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    v2_return = {
-        "data": {
-            "data": {"akey": "avalue"},
-            "metadata": {
-                "created_time": "2018-10-23T20:21:55.042755098Z",
-                "destroyed": False,
-                "version": 13,
-                "deletion_time": "",
-            },
-        }
+@pytest.fixture
+def policies_list_response():
+    return {
+        "policies": ["default", "root", "test-policy"],
     }
 
-    mock_vault.return_value.json.return_value = v2_return
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
+
+@pytest.fixture
+def data_list():
+    return ["foo"]
+
+
+@pytest.fixture
+def read_kv(data):
+    with patch("salt.utils.vault.read_kv", autospec=True) as read:
+        read.return_value = data
+        yield read
+
+
+@pytest.fixture
+def list_kv(data_list):
+    with patch("salt.utils.vault.list_kv", autospec=True) as list:
+        list.return_value = data_list
+        yield list
+
+
+@pytest.fixture
+def read_kv_not_found(read_kv):
+    read_kv.side_effect = vaultutil.VaultNotFoundError
+    yield read_kv
+
+
+@pytest.fixture
+def list_kv_not_found(list_kv):
+    list_kv.side_effect = vaultutil.VaultNotFoundError
+    yield list_kv
+
+
+@pytest.fixture
+def write_kv():
+    with patch("salt.utils.vault.write_kv", autospec=True) as write:
+        yield write
+
+
+@pytest.fixture
+def write_kv_err(write_kv):
+    write_kv.side_effect = vaultutil.VaultPermissionDeniedError("damn")
+    yield write_kv
+
+
+@pytest.fixture
+def patch_kv():
+    with patch("salt.utils.vault.patch_kv", autospec=True) as patch_kv:
+        yield patch_kv
+
+
+@pytest.fixture
+def patch_kv_err(patch_kv):
+    patch_kv.side_effect = vaultutil.VaultPermissionDeniedError("damn")
+    yield patch_kv
+
+
+@pytest.fixture
+def delete_kv():
+    with patch("salt.utils.vault.delete_kv", autospec=True) as delete_kv:
+        yield delete_kv
+
+
+@pytest.fixture
+def delete_kv_err(delete_kv):
+    delete_kv.side_effect = vaultutil.VaultPermissionDeniedError("damn")
+    yield delete_kv
+
+
+@pytest.fixture
+def destroy_kv():
+    with patch("salt.utils.vault.destroy_kv", autospec=True) as destroy_kv:
+        yield destroy_kv
+
+
+@pytest.fixture
+def destroy_kv_err(destroy_kv):
+    destroy_kv.side_effect = vaultutil.VaultPermissionDeniedError("damn")
+    yield destroy_kv
+
+
+@pytest.fixture
+def query():
+    with patch("salt.utils.vault.query", autospec=True) as query:
+        yield query
+
+
+@pytest.mark.parametrize("key,expected", [(None, {"foo": "bar"}), ("foo", "bar")])
+def test_read_secret(read_kv, key, expected):
+    """
+    Ensure read_secret works as expected without and with specified key.
+    KV v1/2 is handled in the utils module.
+    """
+    res = vault.read_secret("some/path", key=key)
+    assert res == expected
+
+
+@pytest.mark.usefixtures("read_kv_not_found", "list_kv_not_found")
+@pytest.mark.parametrize("func", ["read_secret", "list_secrets"])
+def test_read_list_secret_with_default(func):
+    """
+    Ensure read_secret and list_secrets with defaults set return those
+    if the path was not found.
+    """
+    tgt = getattr(vault, func)
+    res = tgt("some/path", default=["f"])
+    assert res == ["f"]
+
+
+@pytest.mark.usefixtures("read_kv_not_found", "list_kv_not_found")
+@pytest.mark.parametrize("func", ["read_secret", "list_secrets"])
+def test_read_list_secret_without_default(func):
+    """
+    Ensure read_secret and list_secrets without defaults set raise
+    a CommandExecutionError when the path is not found.
+    """
+    tgt = getattr(vault, func)
+    with pytest.raises(
+        salt.exceptions.CommandExecutionError, match=".*VaultNotFoundError.*"
     ):
-        vault_return = vault.read_secret("/secret/my/secret", "akey")
-
-    assert vault_return == "avalue"
+        tgt("some/path")
 
 
-def test_read_secret_with_default(path):
-    assert vault.read_secret(path, default="baz") == "baz"
+@pytest.mark.usefixtures("list_kv")
+@pytest.mark.parametrize(
+    "keys_only,expected",
+    [
+        (False, {"keys": ["foo"]}),
+        (True, ["foo"]),
+    ],
+)
+def test_list_secrets(keys_only, expected):
+    """
+    Ensure list_secrets works as expected. keys_only=False is default to
+    stay backwards-compatible. There should not be a reason to have the
+    function return a dict with a single predictable key otherwise.
+    """
+    res = vault.list_secrets("some/path", keys_only=keys_only)
+    assert res == expected
 
 
-def test_read_secret_no_default(path):
-    with pytest.raises(CommandExecutionError):
-        vault.read_secret(path)
+def test_write_secret(data, write_kv):
+    """
+    Ensure write_secret parses kwargs as expected
+    """
+    path = "secret/some/path"
+    res = vault.write_secret(path, **data)
+    assert res
+    write_kv.assert_called_once_with(path, data, opts=ANY, context=ANY)
 
 
-def test_list_secrets_with_default(path):
-    assert vault.list_secrets(path, default=["baz"]) == ["baz"]
+@pytest.mark.usefixtures("write_kv_err")
+def test_write_secret_err(data, caplog):
+    """
+    Ensure write_secret handles exceptions as expected
+    """
+    with caplog.at_level(logging.ERROR):
+        res = vault.write_secret("secret/some/path", **data)
+        assert not res
+        assert (
+            "Failed to write secret! VaultPermissionDeniedError: damn"
+            in caplog.messages
+        )
 
 
-def test_list_secrets_no_default(path):
-    with pytest.raises(CommandExecutionError):
-        vault.list_secrets(path)
+def test_write_raw(data, write_kv):
+    """
+    Ensure write_secret works as expected
+    """
+    path = "secret/some/path"
+    res = vault.write_raw(path, data)
+    assert res
+    write_kv.assert_called_once_with(path, data, opts=ANY, context=ANY)
+
+
+@pytest.mark.usefixtures("write_kv_err")
+def test_write_raw_err(data, caplog):
+    """
+    Ensure write_raw handles exceptions as expected
+    """
+    with caplog.at_level(logging.ERROR):
+        res = vault.write_raw("secret/some/path", data)
+        assert not res
+        assert (
+            "Failed to write secret! VaultPermissionDeniedError: damn"
+            in caplog.messages
+        )
+
+
+def test_patch_secret(data, patch_kv):
+    """
+    Ensure patch_secret parses kwargs as expected
+    """
+    path = "secret/some/path"
+    res = vault.patch_secret(path, **data)
+    assert res
+    patch_kv.assert_called_once_with(path, data, opts=ANY, context=ANY)
+
+
+@pytest.mark.usefixtures("patch_kv_err")
+def test_patch_secret_err(data, caplog):
+    """
+    Ensure patch_secret handles exceptions as expected
+    """
+    with caplog.at_level(logging.ERROR):
+        res = vault.patch_secret("secret/some/path", **data)
+        assert not res
+        assert (
+            "Failed to patch secret! VaultPermissionDeniedError: damn"
+            in caplog.messages
+        )
+
+
+@pytest.mark.parametrize("args", [[], [1, 2]])
+def test_delete_secret(delete_kv, args):
+    """
+    Ensure delete_secret works as expected
+    """
+    path = "secret/some/path"
+    res = vault.delete_secret(path, *args)
+    assert res
+    delete_kv.assert_called_once_with(
+        path, opts=ANY, context=ANY, versions=args or None
+    )
+
+
+@pytest.mark.usefixtures("delete_kv_err")
+@pytest.mark.parametrize("args", [[], [1, 2]])
+def test_delete_secret_err(args, caplog):
+    """
+    Ensure delete_secret handles exceptions as expected
+    """
+    with caplog.at_level(logging.ERROR):
+        res = vault.delete_secret("secret/some/path", *args)
+        assert not res
+        assert (
+            "Failed to delete secret! VaultPermissionDeniedError: damn"
+            in caplog.messages
+        )
+
+
+@pytest.mark.parametrize("args", [[1], [1, 2]])
+def test_destroy_secret(destroy_kv, args):
+    """
+    Ensure destroy_secret works as expected
+    """
+    path = "secret/some/path"
+    res = vault.destroy_secret(path, *args)
+    assert res
+    destroy_kv.assert_called_once_with(path, args, opts=ANY, context=ANY)
+
+
+@pytest.mark.usefixtures("destroy_kv")
+def test_destroy_secret_requires_version():
+    """
+    Ensure destroy_secret requires at least one version
+    """
+    with pytest.raises(
+        salt.exceptions.SaltInvocationError, match=".*at least one version.*"
+    ):
+        vault.destroy_secret("secret/some/path")
+
+
+@pytest.mark.usefixtures("destroy_kv_err")
+@pytest.mark.parametrize("args", [[1], [1, 2]])
+def test_destroy_secret_err(caplog, args):
+    """
+    Ensure destroy_secret handles exceptions as expected
+    """
+    with caplog.at_level(logging.ERROR):
+        res = vault.destroy_secret("secret/some/path", *args)
+        assert not res
+        assert (
+            "Failed to destroy secret! VaultPermissionDeniedError: damn"
+            in caplog.messages
+        )
+
+
+def test_clear_token_cache():
+    """
+    Ensure clear_token_cache wraps the utility function properly
+    """
+    with patch("salt.utils.vault.clear_cache") as cache:
+        vault.clear_token_cache()
+        cache.assert_called_once_with(ANY, ANY, connection=True, session=False)
+
+
+def test_policy_fetch(query, policy_response):
+    """
+    Ensure policy_fetch returns rules only and calls the API as expected
+    """
+    query.return_value = policy_response
+    res = vault.policy_fetch("test-policy")
+    assert res == policy_response["rules"]
+    query.assert_called_once_with(
+        "GET", "sys/policy/test-policy", opts=ANY, context=ANY
+    )
+
+
+def test_policy_fetch_not_found(query):
+    """
+    Ensure policy_fetch returns None when the policy was not found
+    """
+    query.side_effect = vaultutil.VaultNotFoundError
+    res = vault.policy_fetch("test-policy")
+    assert res is None
+
+
+@pytest.mark.parametrize(
+    "func,args",
+    [
+        ("policy_fetch", []),
+        ("policy_write", ["rule"]),
+        ("policy_delete", []),
+        ("policies_list", None),
+    ],
+)
+def test_policy_functions_raise_errors(query, func, args):
+    """
+    Ensure policy functions raise CommandExecutionErrors
+    """
+    query.side_effect = vaultutil.VaultPermissionDeniedError
+    func = getattr(vault, func)
+    with pytest.raises(
+        salt.exceptions.CommandExecutionError, match=".*VaultPermissionDeniedError.*"
+    ):
+        if args is None:
+            func()
+        else:
+            func("test-policy", *args)
+
+
+def test_policy_write(query, policy_response):
+    """
+    Ensure policy_write calls the API as expected
+    """
+    query.return_value = True
+    res = vault.policy_write("test-policy", policy_response["rules"])
+    assert res
+    query.assert_called_once_with(
+        "POST",
+        "sys/policy/test-policy",
+        opts=ANY,
+        context=ANY,
+        payload={"policy": policy_response["rules"]},
+    )
+
+
+def test_policy_delete(query):
+    """
+    Ensure policy_delete calls the API as expected
+    """
+    query.return_value = True
+    res = vault.policy_delete("test-policy")
+    assert res
+    query.assert_called_once_with(
+        "DELETE", "sys/policy/test-policy", opts=ANY, context=ANY
+    )
+
+
+def test_policy_delete_handles_not_found(query):
+    """
+    Ensure policy_delete returns False instead of raising CommandExecutionError
+    when a policy was absent already.
+    """
+    query.side_effect = vaultutil.VaultNotFoundError
+    res = vault.policy_delete("test-policy")
+    assert not res
+
+
+def test_policies_list(query, policies_list_response):
+    """
+    Ensure policies_list returns policy list only and calls the API as expected
+    """
+    query.return_value = policies_list_response
+    res = vault.policies_list()
+    assert res == policies_list_response["policies"]
+    query.assert_called_once_with("GET", "sys/policy", opts=ANY, context=ANY)
+
+
+@pytest.mark.parametrize("method", ["POST", "DELETE"])
+@pytest.mark.parametrize("payload", [None, {"data": {"foo": "bar"}}])
+def test_query(query, method, payload):
+    """
+    Ensure query wraps the utility function properly
+    """
+    query.return_value = True
+    endpoint = "test/endpoint"
+    res = vault.query(method, endpoint, payload=payload)
+    assert res
+    query.assert_called_once_with(
+        method, endpoint, opts=ANY, context=ANY, payload=payload
+    )
+
+
+def test_query_raises_errors(query):
+    """
+    Ensure query raises CommandExecutionErrors
+    """
+    query.side_effect = vaultutil.VaultPermissionDeniedError
+    with pytest.raises(
+        salt.exceptions.CommandExecutionError, match=".*VaultPermissionDeniedError.*"
+    ):
+        vault.query("GET", "test/endpoint")

tests/pytests/unit/pillar/test_vault.py

@@ -1,11 +1,10 @@
-import copy
 import logging
 
 import pytest
-from requests.exceptions import HTTPError
 
 import salt.pillar.vault as vault
-from tests.support.mock import Mock, patch
+import salt.utils.vault as vaultutil
+from tests.support.mock import ANY, Mock, patch
 
 
 @pytest.fixture
@@ -22,93 +21,69 @@ def configure_loader_modules():
 
 
 @pytest.fixture
-def vault_kvv1():
-    res = Mock(status_code=200)
-    res.json.return_value = {"data": {"foo": "bar"}}
-    return Mock(return_value=res)
+def data():
+    return {"foo": "bar"}
 
 
 @pytest.fixture
-def vault_kvv2():
-    res = Mock(status_code=200)
-    res.json.return_value = {"data": {"data": {"foo": "bar"}}, "metadata": {}}
-    return Mock(return_value=res)
+def read_kv(data):
+    with patch("salt.utils.vault.read_kv", autospec=True) as read:
+        read.return_value = data
+        yield read
 
 
 @pytest.fixture
-def is_v2_false():
-    path = "secret/path"
-    return {"v2": False, "data": path, "metadata": path, "delete": path, "type": "kv"}
+def read_kv_not_found(read_kv):
+    read_kv.side_effect = vaultutil.VaultNotFoundError
 
 
 @pytest.fixture
-def is_v2_true():
+def role_a():
     return {
-        "v2": True,
-        "data": "secret/data/path",
-        "metadata": "secret/metadata/path",
-        "type": "kv",
+        "from_db": True,
+        "pass": "hunter2",
+        "list": ["a", "b"],
     }
 
 
-@pytest.mark.parametrize(
-    "is_v2,vaultkv", [("is_v2_false", "vault_kvv1"), ("is_v2_true", "vault_kvv2")]
-)
-def test_ext_pillar(is_v2, vaultkv, request):
-    """
-    Test ext_pillar functionality for KV v1/2
-    """
-    is_v2 = request.getfixturevalue(is_v2)
-    vaultkv = request.getfixturevalue(vaultkv)
-    with patch.dict(
-        vault.__utils__,
-        {"vault.is_v2": Mock(return_value=is_v2), "vault.make_request": vaultkv},
-    ):
-        ext_pillar = vault.ext_pillar("testminion", {}, "path=secret/path")
-        vaultkv.assert_called_once_with("GET", "v1/" + is_v2["data"])
-        assert "foo" in ext_pillar
-        assert "metadata" not in ext_pillar
-        assert "data" not in ext_pillar
-        assert ext_pillar["foo"] == "bar"
+@pytest.fixture
+def role_b():
+    return {
+        "from_web": True,
+        "pass": "hunter1",
+        "list": ["c", "d"],
+    }
 
 
-def test_ext_pillar_not_found(is_v2_false, caplog):
+def test_ext_pillar(read_kv, data):
+    """
+    Test ext_pillar functionality. KV v1/2 is handled by the utils module.
+    """
+    ext_pillar = vault.ext_pillar("testminion", {}, "path=secret/path")
+    read_kv.assert_called_once_with("secret/path", opts=ANY, context=ANY)
+    assert ext_pillar == data
+
+
+@pytest.mark.usefixtures("read_kv_not_found")
+def test_ext_pillar_not_found(caplog):
     """
     Test that HTTP 404 is handled correctly
     """
-    res = Mock(status_code=404, ok=False)
-    res.raise_for_status.side_effect = HTTPError()
     with caplog.at_level(logging.INFO):
-        with patch.dict(
-            vault.__utils__,
-            {
-                "vault.is_v2": Mock(return_value=is_v2_false),
-                "vault.make_request": Mock(return_value=res),
-            },
-        ):
-            ext_pillar = vault.ext_pillar("testminion", {}, "path=secret/path")
-            assert ext_pillar == {}
-            assert "Vault secret not found for: secret/path" in caplog.messages
+        ext_pillar = vault.ext_pillar("testminion", {}, "path=secret/path")
+        assert ext_pillar == {}
+        assert "Vault secret not found for: secret/path" in caplog.messages
 
 
-def test_ext_pillar_nesting_key(is_v2_false, vault_kvv1):
+@pytest.mark.usefixtures("read_kv")
+def test_ext_pillar_nesting_key(data):
     """
     Test that nesting_key is honored as expected
     """
-    with patch.dict(
-        vault.__utils__,
-        {
-            "vault.is_v2": Mock(return_value=is_v2_false),
-            "vault.make_request": vault_kvv1,
-        },
-    ):
-        ext_pillar = vault.ext_pillar(
-            "testminion", {}, "path=secret/path", nesting_key="baz"
-        )
-        assert "foo" not in ext_pillar
-        assert "baz" in ext_pillar
-        assert "foo" in ext_pillar["baz"]
-        assert ext_pillar["baz"]["foo"] == "bar"
+    ext_pillar = vault.ext_pillar(
+        "testminion", {}, "path=secret/path", nesting_key="baz"
+    )
+    assert ext_pillar == {"baz": data}
 
 
 @pytest.mark.parametrize(
@@ -132,78 +107,52 @@ def test_get_paths(pattern, expected):
     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 = [
+@pytest.mark.parametrize(
+    "first,second,expected",
+    [
         (
-            ["salt/roles/db", "salt/roles/web"],
+            "role_a",
+            "role_b",
             {"from_db": True, "from_web": True, "list": ["c", "d"], "pass": "hunter1"},
         ),
         (
-            ["salt/roles/web", "salt/roles/db"],
+            "role_b",
+            "role_a",
             {"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_merging(read_kv, first, second, expected, request):
+    """
+    Test that patterns that result in multiple paths are merged as expected.
+    """
+    first = request.getfixturevalue(first)
+    second = request.getfixturevalue(second)
+    read_kv.side_effect = (first, second)
+    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():
+def test_ext_pillar_disabled_during_pillar_rendering(read_kv):
     """
     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
+    res = vault.ext_pillar(
+        "test-minion", {}, conf="path=secret/path", extra_minion_data=extra
+    )
+    assert res == {}
+    read_kv.assert_not_called()
 
 
+@pytest.mark.usefixtures("read_kv")
 def test_invalid_config(caplog):
     """
     Ensure an empty dict is returned and an error is logged in case

tests/pytests/unit/runners/vault/test_token_auth_deprecated.py

@@ -0,0 +1,151 @@
+"""
+Unit tests for the Vault runner
+
+This module only tests a deprecated function, see
+tests/pytests/unit/runners/test_vault.py for the current tests.
+"""
+
+
+import logging
+
+import pytest
+
+import salt.runners.vault as vault
+import salt.utils.vault as vaultutil
+import salt.utils.vault.client as vclient
+from tests.support.mock import ANY, Mock, patch
+
+pytestmark = [
+    pytest.mark.usefixtures("validate_sig", "policies"),
+]
+
+log = logging.getLogger(__name__)
+
+
+@pytest.fixture
+def configure_loader_modules():
+    return {
+        vault: {
+            "__opts__": {
+                "vault": {
+                    "url": "http://127.0.0.1",
+                    "auth": {
+                        "token": "test",
+                        "method": "token",
+                        "allow_minion_override": True,
+                    },
+                }
+            }
+        }
+    }
+
+
+@pytest.fixture
+def auth():
+    return {
+        "auth": {
+            "client_token": "test",
+            "renewable": False,
+            "lease_duration": 0,
+        }
+    }
+
+
+@pytest.fixture
+def client(auth):
+    client_mock = Mock(vclient.AuthenticatedVaultClient)
+    client_mock.post.return_value = auth
+    with patch("salt.runners.vault._get_master_client", Mock(return_value=client_mock)):
+        yield client_mock
+
+
+@pytest.fixture
+def validate_sig():
+    with patch(
+        "salt.runners.vault._validate_signature", autospec=True, return_value=None
+    ):
+        yield
+
+
+@pytest.fixture
+def policies():
+    with patch("salt.runners.vault._get_policies_cached", autospec=True) as policies:
+        policies.return_value = ["saltstack/minion/test-minion", "saltstack/minions"]
+        yield policies
+
+
+# Basic tests for test_generate_token: all exits
+
+
+def test_generate_token(client):
+    result = vault.generate_token("test-minion", "signature")
+    log.debug("generate_token result: %s", result)
+    assert isinstance(result, dict)
+    assert "error" not in result
+    assert "token" in result
+    assert result["token"] == "test"
+    client.post.assert_called_with("auth/token/create", payload=ANY, wrap=False)
+
+
+def test_generate_token_uses(client):
+    # Test uses
+    num_uses = 6
+    result = vault.generate_token("test-minion", "signature", uses=num_uses)
+    assert "uses" in result
+    assert result["uses"] == num_uses
+    json_request = {
+        "policies": ["saltstack/minion/test-minion", "saltstack/minions"],
+        "num_uses": num_uses,
+        "meta": {
+            "saltstack-jid": "<no jid set>",
+            "saltstack-minion": "test-minion",
+            "saltstack-user": "<no user set>",
+        },
+    }
+    client.post.assert_called_with(
+        "auth/token/create", payload=json_request, wrap=False
+    )
+
+
+def test_generate_token_ttl(client):
+    # Test ttl
+    expected_ttl = "6h"
+    result = vault.generate_token("test-minion", "signature", ttl=expected_ttl)
+    assert result["uses"] == 1
+    json_request = {
+        "policies": ["saltstack/minion/test-minion", "saltstack/minions"],
+        "num_uses": 1,
+        "explicit_max_ttl": expected_ttl,
+        "meta": {
+            "saltstack-jid": "<no jid set>",
+            "saltstack-minion": "test-minion",
+            "saltstack-user": "<no user set>",
+        },
+    }
+    client.post.assert_called_with(
+        "auth/token/create", payload=json_request, wrap=False
+    )
+
+
+def test_generate_token_permission_denied(client):
+    client.post.side_effect = vaultutil.VaultPermissionDeniedError("no reason")
+    result = vault.generate_token("test-minion", "signature")
+    assert isinstance(result, dict)
+    assert "error" in result
+    assert result["error"] == "VaultPermissionDeniedError: no reason"
+
+
+def test_generate_token_exception(client):
+    client.post.side_effect = Exception("Test Exception Reason")
+    result = vault.generate_token("test-minion", "signature")
+    assert isinstance(result, dict)
+    assert "error" in result
+    assert result["error"] == "Exception: Test Exception Reason"
+
+
+def test_generate_token_no_matching_policies(client, policies):
+    policies.return_value = []
+    result = vault.generate_token("test-minion", "signature")
+    assert isinstance(result, dict)
+    assert "error" in result
+    assert result["error"] == "SaltRunnerError: No policies matched minion."

tests/pytests/unit/sdb/test_vault.py

@@ -4,182 +4,138 @@ Test case for the vault SDB module
 
 import pytest
 
+import salt.exceptions
 import salt.sdb.vault as vault
-from tests.support.mock import MagicMock, call, patch
+import salt.utils.vault as vaultutil
+from tests.support.mock import ANY, patch
 
 
 @pytest.fixture
 def configure_loader_modules():
-    return {
-        vault: {
-            "__opts__": {
-                "vault": {
-                    "url": "http://127.0.0.1",
-                    "auth": {"token": "test", "method": "token"},
-                }
-            }
-        }
-    }
+    return {vault: {}}
 
 
-def test_set():
+@pytest.fixture
+def data():
+    return {"bar": "super awesome"}
+
+
+@pytest.fixture
+def read_kv(data):
+    with patch("salt.utils.vault.read_kv", autospec=True) as read:
+        read.return_value = data
+        yield read
+
+
+@pytest.fixture
+def read_kv_not_found(read_kv):
+    read_kv.side_effect = vaultutil.VaultNotFoundError
+
+
+@pytest.fixture
+def read_kv_not_found_once(read_kv, data):
+    read_kv.side_effect = (vaultutil.VaultNotFoundError, data)
+    yield read_kv
+
+
+@pytest.fixture
+def read_kv_err(read_kv):
+    read_kv.side_effect = vaultutil.VaultPermissionDeniedError("damn")
+    yield read_kv
+
+
+@pytest.fixture
+def write_kv():
+    with patch("salt.utils.vault.write_kv", autospec=True) as write:
+        yield write
+
+
+@pytest.fixture
+def write_kv_err(write_kv):
+    write_kv.side_effect = vaultutil.VaultPermissionDeniedError("damn")
+    yield write_kv
+
+
+@pytest.mark.parametrize(
+    "key,exp_path",
+    [
+        ("sdb://myvault/path/to/foo/bar", "path/to/foo"),
+        ("sdb://myvault/path/to/foo?bar", "path/to/foo"),
+    ],
+)
+def test_set(write_kv, key, exp_path, data):
     """
-    Test salt.sdb.vault.set function
+    Test salt.sdb.vault.set_ with current and old (question mark) syntax.
+    KV v1/2 distinction is unnecessary, since that is handled in the utils module.
     """
-    version = {"v2": False, "data": None, "metadata": None, "type": None}
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        vault.set_("sdb://myvault/path/to/foo/bar", "super awesome")
-
-    assert mock_vault.call_args_list == [
-        call(
-            "POST",
-            "v1/sdb://myvault/path/to/foo",
-            json={"bar": "super awesome"},
-        )
-    ]
+    vault.set_(key, "super awesome")
+    write_kv.assert_called_once_with(
+        f"sdb://myvault/{exp_path}", data, opts=ANY, context=ANY
+    )
 
 
-def test_set_v2():
+@pytest.mark.usefixtures("write_kv_err")
+def test_set_err():
     """
-    Test salt.sdb.vault.set function with kv v2 backend
+    Test that salt.sdb.vault.set_ raises CommandExecutionError from other exceptions
     """
-    version = {
-        "v2": True,
-        "data": "path/data/to/foo",
-        "metadata": "path/metadata/to/foo",
-        "type": "kv",
-    }
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        vault.set_("sdb://myvault/path/to/foo/bar", "super awesome")
-
-    assert mock_vault.call_args_list == [
-        call(
-            "POST",
-            "v1/path/data/to/foo",
-            json={"data": {"bar": "super awesome"}},
-        )
-    ]
+    with pytest.raises(salt.exceptions.CommandExecutionError, match="damn") as exc:
+        vault.set_("sdb://myvault/path/to/foo/bar", "foo")
 
 
-def test_set_question_mark():
+@pytest.mark.parametrize(
+    "key,exp_path",
+    [
+        ("sdb://myvault/path/to/foo/bar", "path/to/foo"),
+        ("sdb://myvault/path/to/foo?bar", "path/to/foo"),
+    ],
+)
+def test_get(read_kv, key, exp_path):
     """
-    Test salt.sdb.vault.set_ while using the old
-    deprecated solution with a question mark.
+    Test salt.sdb.vault.get_ with current and old (question mark) syntax.
+    KV v1/2 distinction is unnecessary, since that is handled in the utils module.
     """
-    version = {"v2": False, "data": None, "metadata": None, "type": None}
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        vault.set_("sdb://myvault/path/to/foo?bar", "super awesome")
-
-    assert mock_vault.call_args_list == [
-        call(
-            "POST",
-            "v1/sdb://myvault/path/to/foo",
-            json={"bar": "super awesome"},
-        )
-    ]
-
-
-def test_get():
-    """
-    Test salt.sdb.vault.get function
-    """
-    version = {"v2": False, "data": None, "metadata": None, "type": None}
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    mock_vault.return_value.json.return_value = {"data": {"bar": "test"}}
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        assert vault.get("sdb://myvault/path/to/foo/bar") == "test"
-
-    assert mock_vault.call_args_list == [call("GET", "v1/sdb://myvault/path/to/foo")]
-
-
-def test_get_v2():
-    """
-    Test salt.sdb.vault.get function with kv v2 backend
-    """
-    version = {
-        "v2": True,
-        "data": "path/data/to/foo",
-        "metadata": "path/metadata/to/foo",
-        "type": "kv",
-    }
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    mock_vault.return_value.json.return_value = {"data": {"data": {"bar": "test"}}}
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        assert vault.get("sdb://myvault/path/to/foo/bar") == "test"
-
-    assert mock_vault.call_args_list == [call("GET", "v1/path/data/to/foo")]
-
-
-def test_get_question_mark():
-    """
-    Test salt.sdb.vault.get while using the old
-    deprecated solution with a question mark.
-    """
-    version = {"v2": False, "data": None, "metadata": None, "type": None}
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    mock_vault.return_value.json.return_value = {"data": {"bar": "test"}}
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        assert vault.get("sdb://myvault/path/to/foo?bar") == "test"
-    assert mock_vault.call_args_list == [call("GET", "v1/sdb://myvault/path/to/foo")]
-
-
-def test_get_missing():
-    """
-    Test salt.sdb.vault.get function returns None
-    if vault does not have an entry
-    """
-    version = {"v2": False, "data": None, "metadata": None, "type": None}
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 404
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        assert vault.get("sdb://myvault/path/to/foo/bar") is None
-
-    assert mock_vault.call_args_list == [call("GET", "v1/sdb://myvault/path/to/foo")]
+    res = vault.get(key)
+    assert res == "super awesome"
+    read_kv.assert_called_once_with(f"sdb://myvault/{exp_path}", opts=ANY, context=ANY)
 
 
+@pytest.mark.usefixtures("read_kv")
 def test_get_missing_key():
     """
-    Test salt.sdb.vault.get function returns None
-    if vault does not have the key but does have the entry
+    Test that salt.sdb.vault.get returns None if vault does not have the key
+    but does have the entry.
     """
-    version = {"v2": False, "data": None, "metadata": None, "type": None}
-    mock_version = MagicMock(return_value=version)
-    mock_vault = MagicMock()
-    mock_vault.return_value.status_code = 200
-    mock_vault.return_value.json.return_value = {"data": {"bar": "test"}}
-    with patch.dict(vault.__utils__, {"vault.make_request": mock_vault}), patch.dict(
-        vault.__utils__, {"vault.is_v2": mock_version}
-    ):
-        assert vault.get("sdb://myvault/path/to/foo/foo") is None
+    res = vault.get("sdb://myvault/path/to/foo/foo")
+    assert res is None
 
-    assert mock_vault.call_args_list == [call("GET", "v1/sdb://myvault/path/to/foo")]
+
+@pytest.mark.usefixtures("read_kv_not_found")
+def test_get_missing():
+    """
+    Test that salt.sdb.vault.get returns None if vault does have the entry.
+    """
+    res = vault.get("sdb://myvault/path/to/foo/foo")
+    assert res is None
+
+
+def test_get_whole_dataset(read_kv_not_found_once, data):
+    """
+    Test that salt.sdb.vault.get retries the whole path without key if the
+    first request reported the dataset was not found.
+    """
+    res = vault.get("sdb://myvault/path/to/foo")
+    assert res == data
+    read_kv_not_found_once.assert_called_with(
+        "sdb://myvault/path/to/foo", opts=ANY, context=ANY
+    )
+    assert read_kv_not_found_once.call_count == 2
+
+
+@pytest.mark.usefixtures("read_kv_err")
+def test_get_err():
+    """
+    Test that salt.sdb.vault.get raises CommandExecutionError from other exceptions
+    """
+    with pytest.raises(salt.exceptions.CommandExecutionError, match="damn") as exc:
+        vault.get("sdb://myvault/path/to/foo/bar")

tests/pytests/unit/states/test_vault.py

@@ -0,0 +1,112 @@
+import pytest
+
+import salt.modules.vault as vaultexe
+import salt.states.vault as vault
+from tests.support.mock import Mock, patch
+
+
+@pytest.fixture
+def configure_loader_modules():
+    return {vault: {}}
+
+
+@pytest.fixture
+def policy_fetch():
+    fetch = Mock(return_value="test-rules", spec=vaultexe.policy_fetch)
+    with patch.dict(vault.__salt__, {"vault.policy_fetch": fetch}):
+        yield fetch
+
+
+@pytest.fixture
+def policy_write():
+    write = Mock(return_value=True, spec=vaultexe.policy_write)
+    with patch.dict(vault.__salt__, {"vault.policy_write": write}):
+        yield write
+
+
+@pytest.mark.usefixtures("policy_fetch")
+@pytest.mark.parametrize("test", [False, True])
+def test_policy_present_no_changes(test):
+    """
+    Test that when a policy is present as requested, no changes
+    are reported for success, regardless of opts["test"].
+    """
+    with patch.dict(vault.__opts__, {"test": test}):
+        res = vault.policy_present("test-policy", "test-rules")
+    assert res["result"]
+    assert not res["changes"]
+
+
+@pytest.mark.parametrize("test", [False, True])
+def test_policy_present_create(policy_fetch, policy_write, test):
+    """
+    Test that when a policy does not exist, it will be created.
+    The function should respect opts["test"].
+    """
+    policy_fetch.return_value = None
+    with patch.dict(vault.__opts__, {"test": test}):
+        res = vault.policy_present("test-policy", "test-rules")
+    assert res["changes"]
+    if test:
+        assert res["result"] is None
+        assert "would be created" in res["comment"]
+        policy_write.assert_not_called()
+    else:
+        assert res["result"]
+        assert "has been created" in res["comment"]
+        policy_write.assert_called_once_with("test-policy", "test-rules")
+
+
+@pytest.mark.usefixtures("policy_fetch")
+@pytest.mark.parametrize("test", [False, True])
+def test_policy_present_changes(policy_write, test):
+    """
+    Test that when a policy exists, but the rules need to be updated,
+    it is detected and respects the value of opts["test"].
+    """
+    with patch.dict(vault.__opts__, {"test": test}):
+        res = vault.policy_present("test-policy", "new-test-rules")
+    assert res["changes"]
+    if test:
+        assert res["result"] is None
+        assert "would be updated" in res["comment"]
+        policy_write.assert_not_called()
+    else:
+        assert res["result"]
+        assert "has been updated" in res["comment"]
+        policy_write.assert_called_once_with("test-policy", "new-test-rules")
+
+
+@pytest.mark.parametrize("test", [False, True])
+def test_policy_absent_no_changes(policy_fetch, test):
+    """
+    Test that when a policy is absent as requested, no changes
+    are reported for success, regardless of opts["test"].
+    """
+    policy_fetch.return_value = None
+    with patch.dict(vault.__opts__, {"test": test}):
+        res = vault.policy_absent("test-policy")
+    assert res["result"]
+    assert not res["changes"]
+
+
+@pytest.mark.usefixtures("policy_fetch")
+@pytest.mark.parametrize("test", [False, True])
+def test_policy_absent_changes(test):
+    """
+    Test that when a policy exists, it will be deleted.
+    The function should respect opts["test"].
+    """
+    delete = Mock(spec=vaultexe.policy_delete)
+    with patch.dict(vault.__salt__, {"vault.policy_delete": delete}):
+        with patch.dict(vault.__opts__, {"test": test}):
+            res = vault.policy_absent("test-policy")
+        assert res["changes"]
+        if test:
+            assert res["result"] is None
+            assert "would be deleted" in res["comment"]
+            delete.assert_not_called()
+        else:
+            assert res["result"]
+            assert "has been deleted" in res["comment"]
+            delete.assert_called_once_with("test-policy")

tests/pytests/unit/utils/vault/conftest.py

@@ -0,0 +1,588 @@
+import pytest
+import requests
+
+import salt.modules.event
+import salt.utils.vault as vault
+import salt.utils.vault.auth as vauth
+import salt.utils.vault.client as vclient
+import salt.utils.vault.helpers as hlp
+from tests.support.mock import MagicMock, Mock, patch
+
+
+def _mock_json_response(data, status_code=200, reason=""):
+    """
+    Mock helper for http response
+    """
+    response = Mock(spec=requests.models.Response)
+    response.json.return_value = data
+    response.status_code = status_code
+    response.reason = reason
+    if status_code < 400:
+        response.ok = True
+    else:
+        response.ok = False
+        response.raise_for_status.side_effect = requests.exceptions.HTTPError
+    return response
+
+
+@pytest.fixture(params=[{}])
+def server_config(request):
+    conf = {
+        "url": "http://127.0.0.1:8200",
+        "namespace": None,
+        "verify": None,
+    }
+    conf.update(request.param)
+    return conf
+
+
+@pytest.fixture(params=["token", "approle"])
+def test_config(server_config, request):
+    defaults = {
+        "auth": {
+            "approle_mount": "approle",
+            "approle_name": "salt-master",
+            "method": "token",
+            "secret_id": None,
+            "token_lifecycle": {
+                "minimum_ttl": 10,
+                "renew_increment": None,
+            },
+        },
+        "cache": {
+            "backend": "session",
+            "clear_attempt_revocation": 60,
+            "clear_on_unauthorized": True,
+            "config": 3600,
+            "expire_events": False,
+            "secret": "ttl",
+        },
+        "issue": {
+            "allow_minion_override_params": False,
+            "type": "token",
+            "approle": {
+                "mount": "salt-minions",
+                "params": {
+                    "bind_secret_id": True,
+                    "secret_id_num_uses": 1,
+                    "secret_id_ttl": 60,
+                    "token_explicit_max_ttl": 60,
+                    "token_num_uses": 10,
+                },
+            },
+            "token": {
+                "role_name": None,
+                "params": {
+                    "explicit_max_ttl": None,
+                    "num_uses": 1,
+                },
+            },
+            "wrap": "30s",
+        },
+        "issue_params": {},
+        "metadata": {
+            "entity": {
+                "minion-id": "{minion}",
+            },
+            "token": {
+                "saltstack-jid": "{jid}",
+                "saltstack-minion": "{minion}",
+                "saltstack-user": "{user}",
+            },
+        },
+        "policies": {
+            "assign": [
+                "saltstack/minions",
+                "saltstack/{minion}",
+            ],
+            "cache_time": 60,
+            "refresh_pillar": None,
+        },
+        "server": server_config,
+    }
+
+    if request.param == "token":
+        defaults["auth"]["token"] = "test-token"
+        return defaults
+
+    if request.param == "wrapped_token":
+        defaults["auth"]["method"] = "wrapped_token"
+        defaults["auth"]["token"] = "test-wrapped-token"
+        return defaults
+
+    if request.param == "approle":
+        defaults["auth"]["method"] = "approle"
+        defaults["auth"]["role_id"] = "test-role-id"
+        defaults["auth"]["secret_id"] = "test-secret-id"
+        return defaults
+
+    if request.param == "approle_no_secretid":
+        defaults["auth"]["method"] = "approle"
+        defaults["auth"]["role_id"] = "test-role-id"
+        return defaults
+
+
+@pytest.fixture(params=["token", "approle"])
+def test_remote_config(server_config, request):
+    defaults = {
+        "auth": {
+            "approle_mount": "approle",
+            "approle_name": "salt-master",
+            "method": "token",
+            "secret_id": None,
+            "token_lifecycle": {
+                "minimum_ttl": 10,
+                "renew_increment": None,
+            },
+        },
+        "cache": {
+            "backend": "session",
+            "clear_attempt_revocation": 60,
+            "clear_on_unauthorized": True,
+            "config": 3600,
+            "expire_events": False,
+            "kv_metadata": "connection",
+            "secret": "ttl",
+        },
+        "server": server_config,
+    }
+
+    if request.param == "token":
+        defaults["auth"]["token"] = "test-token"
+        return defaults
+
+    if request.param == "wrapped_token":
+        defaults["auth"]["method"] = "wrapped_token"
+        defaults["auth"]["token"] = "test-wrapped-token"
+        return defaults
+
+    if request.param == "token_changed":
+        defaults["auth"]["token"] = "test-token-changed"
+        return defaults
+
+    if request.param == "approle":
+        defaults["auth"]["method"] = "approle"
+        defaults["auth"]["role_id"] = "test-role-id"
+        # actual remote config would not contain secret_id, but
+        # this is used for testing both from local and from remote
+        defaults["auth"]["secret_id"] = "test-secret-id"
+        return defaults
+
+    if request.param == "approle_no_secretid":
+        defaults["auth"]["method"] = "approle"
+        defaults["auth"]["role_id"] = "test-role-id"
+        return defaults
+
+    # this happens when wrapped role_ids are merged by _query_master
+    if request.param == "approle_wrapped_roleid":
+        defaults["auth"]["method"] = "approle"
+        defaults["auth"]["role_id"] = {"role_id": "test-role-id"}
+        # actual remote config does not contain secret_id
+        defaults["auth"]["secret_id"] = True
+        return defaults
+
+
+@pytest.fixture
+def role_id_response():
+    return {
+        "request_id": "c85838c5-ecfe-6d07-4b28-1935ac2e304a",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": {"role_id": "58b4c650-3d13-5932-a2fa-03865c8e85d7"},
+        "warnings": None,
+    }
+
+
+@pytest.fixture
+def secret_id_response():
+    return {
+        "request_id": "c85838c5-ecfe-6d07-4b28-1935ac2e304a",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": {
+            "secret_id_accessor": "84896a0c-1347-aa90-a4f6-aca8b7558780",
+            "secret_id": "841771dc-11c9-bbc7-bcac-6a3945a69cd9",
+            "secret_id_ttl": 1337,
+        },
+        "warnings": None,
+    }
+
+
+@pytest.fixture
+def secret_id_meta_response():
+    return {
+        "request_id": "7c97d03d-2166-6217-8da1-19604febae5c",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": {
+            "cidr_list": [],
+            "creation_time": "2022-08-22T17:37:07.753989459+00:00",
+            "expiration_time": "2339-07-13T13:23:46.753989459+00:00",
+            "last_updated_time": "2022-08-22T17:37:07.753989459+00:00",
+            "metadata": {},
+            "secret_id_accessor": "b1c88755-f2f5-2fd2-4bcc-cade95f6ba96",
+            "secret_id_num_uses": 0,
+            "secret_id_ttl": 9999999999,
+            "token_bound_cidrs": [],
+        },
+        "warnings": None,
+    }
+
+
+@pytest.fixture
+def wrapped_role_id_response():
+    return {
+        "request_id": "",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": None,
+        "warnings": None,
+        "wrap_info": {
+            "token": "test-wrapping-token",
+            "accessor": "test-wrapping-token-accessor",
+            "ttl": 180,
+            "creation_time": "2022-09-10T13:37:12.123456789+00:00",
+            "creation_path": "auth/approle/role/test-minion/role-id",
+            "wrapped_accessor": "",
+        },
+    }
+
+
+@pytest.fixture
+def wrapped_secret_id_response():
+    return {
+        "request_id": "",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": None,
+        "warnings": None,
+        "wrap_info": {
+            "token": "test-wrapping-token",
+            "accessor": "test-wrapping-token-accessor",
+            "ttl": 180,
+            "creation_time": "2022-09-10T13:37:12.123456789+00:00",
+            "creation_path": "auth/approle/role/test-minion/secret-id",
+            "wrapped_accessor": "",
+        },
+    }
+
+
+@pytest.fixture
+def wrapped_role_id_lookup_response():
+    return {
+        "request_id": "31e7020e-3ce3-2c63-e453-d5da8a9890f1",
+        "lease_id": "",
+        "renewable": False,
+        "lease_duration": 0,
+        "data": {
+            "creation_path": "auth/approle/role/test-minion/role-id",
+            "creation_time": "2022-09-10T13:37:12.123456789+00:00",
+            "creation_ttl": 180,
+        },
+        "wrap_info": None,
+        "warnings": None,
+        "auth": None,
+    }
+
+
+@pytest.fixture
+def wrapped_token_auth_response():
+    return {
+        "request_id": "",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": None,
+        "warnings": None,
+        "wrap_info": {
+            "token": "test-wrapping-token",
+            "accessor": "test-wrapping-token-accessor",
+            "ttl": 180,
+            "creation_time": "2022-09-10T13:37:12.123456789+00:00",
+            "creation_path": "auth/token/create/salt-minion",
+            "wrapped_accessor": "",
+        },
+    }
+
+
+@pytest.fixture
+def token_lookup_self_response():
+    return {
+        "request_id": "0e8c388e-2cb6-bcb2-83b7-625127d568bb",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": {
+            "accessor": "test-token-accessor",
+            "creation_time": 1661188581,
+            "creation_ttl": 9999999999,
+            "display_name": "",
+            "entity_id": "",
+            "expire_time": "2339-07-13T11:03:00.473212541+00:00",
+            "explicit_max_ttl": 0,
+            "id": "test-token",
+            "issue_time": "2022-08-22T17:16:21.473219641+00:00",
+            "meta": {},
+            "num_uses": 0,
+            "orphan": True,
+            "path": "",
+            "policies": ["default"],
+            "renewable": True,
+            "ttl": 9999999999,
+            "type": "service",
+        },
+        "warnings": None,
+    }
+
+
+@pytest.fixture
+def token_renew_self_response():
+    return {
+        "auth": {
+            "client_token": "test-token",
+            "policies": ["default", "renewed"],
+            "metadata": {},
+        },
+        "lease_duration": 3600,
+        "renewable": True,
+    }
+
+
+@pytest.fixture
+def token_renew_other_response():
+    return {
+        "auth": {
+            "client_token": "other-test-token",
+            "policies": ["default", "renewed"],
+            "metadata": {},
+        },
+        "lease_duration": 3600,
+        "renewable": True,
+    }
+
+
+@pytest.fixture
+def token_renew_accessor_response():
+    return {
+        "auth": {
+            "client_token": "",
+            "policies": ["default", "renewed"],
+            "metadata": {},
+        },
+        "lease_duration": 3600,
+        "renewable": True,
+    }
+
+
+@pytest.fixture
+def token_auth():
+    return {
+        "request_id": "0e8c388e-2cb6-bcb2-83b7-625127d568bb",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "auth": {
+            "client_token": "test-token",
+            "renewable": True,
+            "lease_duration": 9999999999,
+            "num_uses": 0,
+            "creation_time": 1661188581,
+        },
+    }
+
+
+@pytest.fixture
+def lease_response():
+    return {
+        "request_id": "0e8c388e-2cb6-bcb2-83b7-625127d568bb",
+        "lease_id": "database/creds/testrole/abcd",
+        "lease_duration": 1337,
+        "renewable": True,
+        "data": {
+            "username": "test",
+            "password": "test",
+        },
+    }
+
+
+@pytest.fixture
+def lease():
+    return {
+        "id": "database/creds/testrole/abcd",
+        "lease_id": "database/creds/testrole/abcd",
+        "renewable": True,
+        "duration": 1337,
+        "creation_time": 0,
+        "expire_time": 1337,
+        "data": {
+            "username": "test",
+            "password": "test",
+        },
+    }
+
+
+@pytest.fixture
+def session():
+    return Mock(spec=requests.Session)
+
+
+@pytest.fixture
+def req(session):
+    yield session.request
+
+
+@pytest.fixture
+def req_failed(req, request):
+    status_code = getattr(request, "param", 502)
+    req.return_value = _mock_json_response({"errors": ["foo"]}, status_code=status_code)
+    yield req
+
+
+@pytest.fixture
+def req_success(req):
+    req.return_value = _mock_json_response(None, status_code=204)
+    yield req
+
+
+@pytest.fixture(params=[200])
+def req_any(req, request):
+    data = {}
+    if request.param != 204:
+        data["data"] = {"foo": "bar"}
+    if request.param >= 400:
+        data["errors"] = ["foo"]
+    req.return_value = _mock_json_response(data, status_code=request.param)
+    yield req
+
+
+@pytest.fixture
+def req_unwrapping(wrapped_role_id_lookup_response, role_id_response, req):
+    req.side_effect = (
+        lambda method, url, **kwargs: _mock_json_response(
+            wrapped_role_id_lookup_response
+        )
+        if url.endswith("sys/wrapping/lookup")
+        else _mock_json_response(role_id_response)
+    )
+    yield req
+
+
+@pytest.fixture(params=["data"])
+def unauthd_client_mock(server_config, request):
+    client = Mock(spec=vclient.VaultClient)
+    client.get_config.return_value = server_config
+    client.unwrap.return_value = {request.param: {"bar": "baz"}}
+    yield client
+
+
+@pytest.fixture(params=[None, "valid_token"])
+def client(server_config, request, session):
+    if request.param is None:
+        return vclient.VaultClient(**server_config, session=session)
+    if request.param == "valid_token":
+        token = request.getfixturevalue(request.param)
+        auth = Mock(spec=vauth.VaultTokenAuth)
+        auth.is_renewable.return_value = True
+        auth.is_valid.return_value = True
+        auth.get_token.return_value = token
+        return vclient.AuthenticatedVaultClient(auth, **server_config, session=session)
+    if request.param == "invalid_token":
+        token = request.getfixturevalue(request.param)
+        auth = Mock(spec=vauth.VaultTokenAuth)
+        auth.is_renewable.return_value = True
+        auth.is_valid.return_value = False
+        auth.get_token.side_effect = vault.VaultAuthExpired
+        return vclient.AuthenticatedVaultClient(auth, **server_config, session=session)
+
+
+@pytest.fixture
+def valid_token(token_auth):
+    token = MagicMock(spec=vault.VaultToken, **token_auth["auth"])
+    token.is_valid.return_value = True
+    token.is_renewable.return_value = True
+    token.payload.return_value = {"token": token_auth["auth"]["client_token"]}
+    token.__str__.return_value = token_auth["auth"]["client_token"]
+    token.to_dict.return_value = token_auth["auth"]
+    return token
+
+
+@pytest.fixture
+def invalid_token(valid_token):
+    valid_token.is_valid.return_value = False
+    valid_token.is_renewable.return_value = False
+    return valid_token
+
+
+@pytest.fixture
+def cache_factory():
+    with patch("salt.cache.factory", autospec=True) as factory:
+        yield factory
+
+
+@pytest.fixture
+def events():
+    return Mock(spec=salt.modules.event.send)
+
+
+@pytest.fixture(
+    params=["MASTER", "MASTER_IMPERSONATING", "MINION_LOCAL", "MINION_REMOTE"]
+)
+def salt_runtype(request):
+    runtype = Mock(spec=hlp._get_salt_run_type)
+    runtype.return_value = getattr(hlp, f"SALT_RUNTYPE_{request.param}")
+    with patch("salt.utils.vault.helpers._get_salt_run_type", runtype):
+        yield
+
+
+@pytest.fixture(
+    params=[
+        "master",
+        "master_impersonating",
+        "minion_local_1",
+        "minion_local_2",
+        "minion_local_3",
+        "minion_remote",
+    ]
+)
+def opts_runtype(request):
+    return {
+        "master": {
+            "__role": "master",
+            "vault": {},
+        },
+        "master_peer_run": {
+            "__role": "master",
+            "grains": {
+                "id": "test-minion",
+            },
+            "vault": {},
+        },
+        "master_impersonating": {
+            "__role": "master",
+            "minion_id": "test-minion",
+            "grains": {
+                "id": "test-minion",
+            },
+            "vault": {},
+        },
+        "minion_local_1": {
+            "grains": {"id": "test-minion"},
+            "local": True,
+        },
+        "minion_local_2": {
+            "file_client": "local",
+            "grains": {"id": "test-minion"},
+        },
+        "minion_local_3": {
+            "grains": {"id": "test-minion"},
+            "master_type": "disable",
+        },
+        "minion_remote": {
+            "grains": {"id": "test-minion"},
+        },
+    }[request.param]

tests/pytests/unit/utils/vault/test_api.py

@@ -0,0 +1,401 @@
+import pytest
+
+import salt.utils.vault as vaultutil
+import salt.utils.vault.api as vapi
+import salt.utils.vault.client as vclient
+from tests.support.mock import Mock, patch
+
+
+@pytest.fixture
+def entity_lookup_response():
+    return {
+        "data": {
+            "aliases": [],
+            "creation_time": "2017-11-13T21:01:33.543497Z",
+            "direct_group_ids": [],
+            "group_ids": [],
+            "id": "043fedec-967d-b2c9-d3af-0c467b04e1fd",
+            "inherited_group_ids": [],
+            "last_update_time": "2017-11-13T21:01:33.543497Z",
+            "merged_entity_ids": None,
+            "metadata": None,
+            "name": "test-minion",
+            "policies": None,
+        }
+    }
+
+
+@pytest.fixture
+def entity_fetch_response():
+    return {
+        "data": {
+            "aliases": [],
+            "creation_time": "2018-09-19T17:20:27.705389973Z",
+            "direct_group_ids": [],
+            "disabled": False,
+            "group_ids": [],
+            "id": "test-entity-id",
+            "inherited_group_ids": [],
+            "last_update_time": "2018-09-19T17:20:27.705389973Z",
+            "merged_entity_ids": None,
+            "metadata": {
+                "minion-id": "test-minion",
+            },
+            "name": "salt_minion_test-minion",
+            "policies": [
+                "default",
+                "saltstack/minions",
+                "saltstack/minion/test-minion",
+            ],
+        }
+    }
+
+
+@pytest.fixture
+def secret_id_response():
+    return {
+        "request_id": "0e8c388e-2cb6-bcb2-83b7-625127d568bb",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": {
+            "secret_id_accessor": "84896a0c-1347-aa90-a4f6-aca8b7558780",
+            "secret_id": "841771dc-11c9-bbc7-bcac-6a3945a69cd9",
+            "secret_id_ttl": 60,
+        },
+    }
+
+
+@pytest.fixture
+def secret_id_lookup_accessor_response():
+    return {
+        "request_id": "28f2f9fb-26c0-6022-4970-baeb6366b085",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": {
+            "cidr_list": [],
+            "creation_time": "2022-09-09T15:11:28.358490481+00:00",
+            "expiration_time": "2022-10-11T15:11:28.358490481+00:00",
+            "last_updated_time": "2022-09-09T15:11:28.358490481+00:00",
+            "metadata": {},
+            "secret_id_accessor": "0380eb9f-3041-1c1c-234c-fde31a1a1fc1",
+            "secret_id_num_uses": 1,
+            "secret_id_ttl": 9999999999,
+            "token_bound_cidrs": [],
+        },
+        "warnings": None,
+    }
+
+
+@pytest.fixture
+def wrapped_response():
+    return {
+        "request_id": "",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": None,
+        "warnings": None,
+        "wrap_info": {
+            "token": "test-wrapping-token",
+            "accessor": "test-wrapping-token-accessor",
+            "ttl": 180,
+            "creation_time": "2022-09-10T13:37:12.123456789+00:00",
+            "creation_path": "whatever/not/checked/here",
+            "wrapped_accessor": "84896a0c-1347-aa90-a4f6-aca8b7558780",
+        },
+    }
+
+
+@pytest.fixture
+def approle_meta(secret_id_serialized):
+    return {
+        "bind_secret_id": True,
+        "local_secret_ids": False,
+        "secret_id_bound_cidrs": [],
+        "secret_id_num_uses": secret_id_serialized["secret_id_num_uses"],
+        "secret_id_ttl": secret_id_serialized["secret_id_ttl"],
+        "token_bound_cidrs": [],
+        "token_explicit_max_ttl": 9999999999,
+        "token_max_ttl": 0,
+        "token_no_default_policy": False,
+        "token_num_uses": 1,
+        "token_period": 0,
+        "token_policies": ["default"],
+        "token_ttl": 0,
+        "token_type": "default",
+    }
+
+
+@pytest.fixture
+def secret_id_serialized(secret_id_response):
+    return {
+        "secret_id": secret_id_response["data"]["secret_id"],
+        "secret_id_ttl": secret_id_response["data"]["secret_id_ttl"],
+        "secret_id_num_uses": 1,
+        # + creation_time
+        # + expire_time
+    }
+
+
+@pytest.fixture
+def lookup_mount_response():
+    return {
+        "request_id": "7a49be19-199b-ce19-c139-0c334bf07d72",
+        "lease_id": "",
+        "lease_duration": 0,
+        "renewable": False,
+        "data": {
+            "accessor": "auth_approle_cafebabe",
+            "config": {
+                "allowed_response_headers": [""],
+                "audit_non_hmac_request_keys": [""],
+                "audit_non_hmac_response_keys": [""],
+                "default_lease_ttl": 0,
+                "force_no_cache": False,
+                "max_lease_ttl": 0,
+                "passthrough_request_headers": [""],
+                "token_type": "default-service",
+            },
+            "deprecation_status": "supported",
+            "description": "",
+            "external_entropy_access": False,
+            "local": False,
+            "options": None,
+            "plugin_version": "",
+            "running_plugin_version": "v1.13.1+builtin.vault",
+            "running_sha256": "",
+            "seal_wrap": False,
+            "type": "approle",
+            "uuid": "testuuid",
+        },
+        "warnings": None,
+    }
+
+
+@pytest.fixture
+def client():
+    yield Mock(spec=vclient.AuthenticatedVaultClient)
+
+
+@pytest.fixture
+def approle_api(client):
+    yield vapi.AppRoleApi(client)
+
+
+@pytest.fixture
+def identity_api(client):
+    yield vapi.IdentityApi(client)
+
+
+def test_list_approles(approle_api, client):
+    """
+    Ensure list_approles call the API as expected and returns only a list of names
+    """
+    client.list.return_value = {"data": {"keys": ["foo", "bar"]}}
+    res = approle_api.list_approles(mount="salt-minions")
+    assert res == ["foo", "bar"]
+    client.list.assert_called_once_with("auth/salt-minions/role")
+
+
+def test_destroy_secret_id_by_secret_id(approle_api, client):
+    """
+    Ensure destroy_secret_id calls the API as expected.
+    """
+    approle_api.destroy_secret_id(
+        "test-minion", secret_id="test-secret-id", mount="salt-minions"
+    )
+    client.post.assert_called_once_with(
+        "auth/salt-minions/role/test-minion/secret-id/destroy",
+        payload={"secret_id": "test-secret-id"},
+    )
+
+
+def test_destroy_secret_id_by_accessor(approle_api, client):
+    """
+    Ensure destroy_secret_id calls the API as expected.
+    """
+    approle_api.destroy_secret_id(
+        "test-minion", accessor="test-accessor", mount="salt-minions"
+    )
+    client.post.assert_called_once_with(
+        "auth/salt-minions/role/test-minion/secret-id-accessor/destroy",
+        payload={"secret_id_accessor": "test-accessor"},
+    )
+
+
+@pytest.mark.parametrize(
+    "aliases",
+    [
+        [],
+        [
+            {"mount_accessor": "test-accessor", "id": "test-entity-alias-id"},
+            {"mount_accessor": "other-accessor", "id": "other-entity-alias-id"},
+        ],
+    ],
+)
+def test_write_entity_alias(client, aliases, entity_fetch_response, identity_api):
+    """
+    Ensure write_entity_alias calls the API as expected.
+    """
+    metadata = {"foo": "bar"}
+    payload = {
+        "canonical_id": "test-entity-id",
+        "mount_accessor": "test-accessor",
+        "name": "test-role-id",
+        "custom_metadata": metadata,
+    }
+    if aliases:
+        entity_fetch_response["data"]["aliases"] = aliases
+        if aliases[0]["mount_accessor"] == "test-accessor":
+            payload["id"] = aliases[0]["id"]
+
+    with patch(
+        "salt.utils.vault.api.IdentityApi._lookup_mount_accessor",
+        return_value="test-accessor",
+    ), patch(
+        "salt.utils.vault.api.IdentityApi.read_entity",
+        return_value=entity_fetch_response["data"],
+    ):
+        identity_api.write_entity_alias(
+            "salt_minion_test-minion",
+            alias_name="test-role-id",
+            mount="salt-minions",
+            custom_metadata=metadata,
+        )
+        client.post.assert_called_with("identity/entity-alias", payload=payload)
+
+
+def test_write_entity(client, identity_api):
+    """
+    Ensure write_entity calls the API as expected.
+    """
+    metadata = {"foo": "bar"}
+    identity_api.write_entity("salt_minion_test-minion", metadata=metadata)
+    payload = {"metadata": metadata}
+    client.post.assert_called_with(
+        "identity/entity/name/salt_minion_test-minion", payload=payload
+    )
+
+
+def test_read_entity_by_alias_failed(client, identity_api):
+    """
+    Ensure read_entity_by_alias raises VaultNotFoundError if the lookup fails.
+    """
+    with patch(
+        "salt.utils.vault.api.IdentityApi._lookup_mount_accessor",
+        return_value="test-accessor",
+    ):
+        client.post.return_value = []
+        with pytest.raises(vapi.VaultNotFoundError):
+            identity_api.read_entity_by_alias(
+                alias="test-role-id", mount="salt-minions"
+            )
+
+
+def test_read_entity_by_alias(client, entity_lookup_response, identity_api):
+    """
+    Ensure read_entity_by_alias calls the API as expected.
+    """
+    with patch(
+        "salt.utils.vault.api.IdentityApi._lookup_mount_accessor",
+        return_value="test-accessor",
+    ):
+        client.post.return_value = entity_lookup_response
+        res = identity_api.read_entity_by_alias(
+            alias="test-role-id", mount="salt-minions"
+        )
+        assert res == entity_lookup_response["data"]
+        payload = {
+            "alias_name": "test-role-id",
+            "alias_mount_accessor": "test-accessor",
+        }
+        client.post.assert_called_once_with("identity/lookup/entity", payload=payload)
+
+
+def test_lookup_mount_accessor(client, identity_api, lookup_mount_response):
+    """
+    Ensure _lookup_mount_accessor calls the API as expected.
+    """
+    client.get.return_value = lookup_mount_response
+    res = identity_api._lookup_mount_accessor("salt-minions")
+    client.get.assert_called_once_with("sys/auth/salt-minions")
+    assert res == "auth_approle_cafebabe"
+
+
+@pytest.mark.parametrize("wrap", ["30s", False])
+def test_generate_secret_id(
+    client, wrapped_response, secret_id_response, wrap, approle_api
+):
+    """
+    Ensure generate_secret_id calls the API as expected.
+    """
+
+    def res_or_wrap(*args, **kwargs):
+        if kwargs.get("wrap"):
+            return vaultutil.VaultWrappedResponse(**wrapped_response["wrap_info"])
+        return secret_id_response
+
+    client.post.side_effect = res_or_wrap
+    metadata = {"foo": "bar"}
+    res = approle_api.generate_secret_id(
+        "test-minion", mount="salt-minions", metadata=metadata, wrap=wrap
+    )
+    if wrap:
+        assert res == vaultutil.VaultWrappedResponse(**wrapped_response["wrap_info"])
+    else:
+        assert res == vaultutil.VaultSecretId(**secret_id_response["data"])
+    client.post.assert_called_once_with(
+        "auth/salt-minions/role/test-minion/secret-id",
+        payload={"metadata": '{"foo": "bar"}'},
+        wrap=wrap,
+    )
+
+
+@pytest.mark.parametrize("wrap", ["30s", False])
+def test_read_role_id(client, wrapped_response, wrap, approle_api):
+    """
+    Ensure read_role_id calls the API as expected.
+    """
+
+    def res_or_wrap(*args, **kwargs):
+        if kwargs.get("wrap"):
+            return vaultutil.VaultWrappedResponse(**wrapped_response["wrap_info"])
+        return {"data": {"role_id": "test-role-id"}}
+
+    client.get.side_effect = res_or_wrap
+    res = approle_api.read_role_id("test-minion", mount="salt-minions", wrap=wrap)
+    if wrap:
+        assert res == vaultutil.VaultWrappedResponse(**wrapped_response["wrap_info"])
+    else:
+        assert res == "test-role-id"
+    client.get.assert_called_once_with(
+        "auth/salt-minions/role/test-minion/role-id", wrap=wrap
+    )
+
+
+def test_read_approle(client, approle_api, approle_meta):
+    """
+    Ensure read_approle calls the API as expected.
+    """
+    client.get.return_value = {"data": approle_meta}
+    res = approle_api.read_approle("test-minion", mount="salt-minions")
+    assert res == approle_meta
+    client.get.assert_called_once_with("auth/salt-minions/role/test-minion")
+
+
+def test_write_approle(approle_api, client):
+    """
+    Ensure _manage_approle calls the API as expected.
+    """
+    policies = {"foo": "bar"}
+    payload = {
+        "token_explicit_max_ttl": 9999999999,
+        "token_num_uses": 1,
+        "token_policies": policies,
+    }
+    approle_api.write_approle("test-minion", mount="salt-minions", **payload)
+    client.post.assert_called_once_with(
+        "auth/salt-minions/role/test-minion", payload=payload
+    )

tests/pytests/unit/utils/vault/test_auth.py

@@ -0,0 +1,312 @@
+import pytest
+
+import salt.utils.vault as vault
+import salt.utils.vault.auth as vauth
+import salt.utils.vault.cache as vcache
+import salt.utils.vault.client as vclient
+import salt.utils.vault.leases as vleases
+from tests.support.mock import Mock, patch
+
+
+@pytest.fixture
+def token(token_auth):
+    return vleases.VaultToken(**token_auth["auth"])
+
+
+@pytest.fixture
+def token_invalid(token_auth):
+    token_auth["auth"]["num_uses"] = 1
+    token_auth["auth"]["use_count"] = 1
+    return vleases.VaultToken(**token_auth["auth"])
+
+
+@pytest.fixture
+def token_unrenewable(token_auth):
+    token_auth["auth"]["renewable"] = False
+    return vleases.VaultToken(**token_auth["auth"])
+
+
+@pytest.fixture
+def secret_id(secret_id_response):
+    return vleases.VaultSecretId(**secret_id_response["data"])
+
+
+@pytest.fixture
+def secret_id_invalid(secret_id_response):
+    secret_id_response["data"]["secret_id_num_uses"] = 1
+    secret_id_response["data"]["use_count"] = 1
+    return vleases.VaultSecretId(**secret_id_response["data"])
+
+
+@pytest.fixture(params=["secret_id"])
+def approle(request):
+    secret_id = request.param
+    if secret_id is not None:
+        secret_id = request.getfixturevalue(secret_id)
+    return vauth.VaultAppRole("test-role-id", secret_id)
+
+
+@pytest.fixture
+def approle_invalid(secret_id_invalid):
+    return vauth.VaultAppRole("test-role-id", secret_id_invalid)
+
+
+@pytest.fixture
+def token_store(token):
+    store = Mock(spec=vauth.VaultTokenAuth)
+    store.is_valid.return_value = True
+    store.get_token.return_value = token
+    return store
+
+
+@pytest.fixture
+def token_store_empty(token_store):
+    token_store.is_valid.return_value = False
+    token_store.get_token.side_effect = vault.VaultAuthExpired
+    return token_store
+
+
+@pytest.fixture
+def token_store_empty_first(token_store, token):
+    token_store.is_valid.side_effect = (False, True)
+    token_store.get_token.side_effect = (token, vault.VaultException)
+    return token_store
+
+
+@pytest.fixture
+def uncached():
+    cache = Mock(spec=vcache.VaultAuthCache)
+    cache.exists.return_value = False
+    cache.get.return_value = None
+    return cache
+
+
+@pytest.fixture
+def cached_token(uncached, token):
+    uncached.exists.return_value = True
+    uncached.get.return_value = token
+    return uncached
+
+
+@pytest.fixture
+def client(token_auth):
+    token_auth["auth"]["client_token"] = "new-test-token"
+    client = Mock(spec=vclient.VaultClient)
+    client.post.return_value = token_auth
+    return client
+
+
+def test_token_auth_uninitialized(uncached):
+    """
+    Test that an exception is raised when a token is requested
+    and the authentication container was not passed a valid token.
+    """
+    auth = vauth.VaultTokenAuth(cache=uncached)
+    uncached.get.assert_called_once()
+    assert auth.is_valid() is False
+    assert auth.is_renewable() is False
+    auth.used()
+    with pytest.raises(vault.VaultAuthExpired):
+        auth.get_token()
+
+
+def test_token_auth_cached(cached_token, token):
+    """
+    Test that tokens are read from cache.
+    """
+    auth = vauth.VaultTokenAuth(cache=cached_token)
+    assert auth.is_valid()
+    assert auth.get_token() == token
+
+
+def test_token_auth_invalid_token(invalid_token):
+    """
+    Test that an exception is raised when a token is requested
+    and the container's token is invalid.
+    """
+    auth = vauth.VaultTokenAuth(token=invalid_token)
+    assert auth.is_valid() is False
+    assert auth.is_renewable() is False
+    with pytest.raises(vault.VaultAuthExpired):
+        auth.get_token()
+
+
+def test_token_auth_unrenewable_token(token_unrenewable):
+    """
+    Test that it is reported correctly by the container
+    when a token is not renewable.
+    """
+    auth = vauth.VaultTokenAuth(token=token_unrenewable)
+    assert auth.is_valid() is True
+    assert auth.is_renewable() is False
+    assert auth.get_token() == token_unrenewable
+
+
+@pytest.mark.parametrize("num_uses", [0, 1, 10])
+def test_token_auth_used_num_uses(uncached, token, num_uses):
+    """
+    Ensure that cache writes for use count are only done when
+    num_uses is not 0 (= unlimited).
+    Single-use tokens still require cache writes for updating
+    ``uses``. The cache cannot be flushed here since
+    exceptions might be used to indicate the token expiry
+    to factory methods.
+    """
+    token = token.with_renewed(num_uses=num_uses)
+    auth = vauth.VaultTokenAuth(cache=uncached, token=token)
+    auth.used()
+    if num_uses > 0:
+        uncached.store.assert_called_once_with(token)
+    else:
+        uncached.store.assert_not_called()
+
+
+@pytest.mark.parametrize("num_uses", [0, 1, 10])
+def test_token_auth_update_token(uncached, token, num_uses):
+    """
+    Ensure that partial updates to the token in use are possible
+    and that the cache writes are independent from num_uses.
+    Also ensure the token is treated as immutable
+    """
+    auth = vauth.VaultTokenAuth(cache=uncached, token=token)
+    old_token = token
+    old_token_ttl = old_token.duration
+    auth.update_token({"num_uses": num_uses, "ttl": 8483})
+    updated_token = token.with_renewed(num_uses=num_uses, ttl=8483)
+    assert auth.token == updated_token
+    assert old_token.duration == old_token_ttl
+    uncached.store.assert_called_once_with(updated_token)
+
+
+def test_token_auth_replace_token(uncached, token):
+    """
+    Ensure completely replacing the token is possible and
+    results in a cache write. This is important when an
+    InvalidVaultToken has to be replaced with a VaultToken,
+    eg by a different authentication method.
+    """
+    auth = vauth.VaultTokenAuth(cache=uncached)
+    assert isinstance(auth.token, vauth.InvalidVaultToken)
+    auth.replace_token(token)
+    assert isinstance(auth.token, vleases.VaultToken)
+    assert auth.token == token
+    uncached.store.assert_called_once_with(token)
+
+
+@pytest.mark.parametrize("token", [False, True])
+@pytest.mark.parametrize("approle", [False, True])
+def test_approle_auth_is_valid(token, approle):
+    """
+    Test that is_valid reports true when either the token
+    or the secret ID is valid
+    """
+    token = Mock(spec=vleases.VaultToken)
+    token.is_valid.return_value = token
+    approle = Mock(spec=vleases.VaultSecretId)
+    approle.is_valid.return_value = approle
+    auth = vauth.VaultAppRoleAuth(approle, None, token_store=token)
+    assert auth.is_valid() is (token or approle)
+
+
+def test_approle_auth_get_token_store_available(token_store, approle, token):
+    """
+    Ensure no login attempt is made when a cached token is available
+    """
+    auth = vauth.VaultAppRoleAuth(approle, None, token_store=token_store)
+    with patch("salt.utils.vault.auth.VaultAppRoleAuth._login") as login:
+        res = auth.get_token()
+        login.assert_not_called()
+        assert res == token
+
+
+def test_approle_auth_get_token_store_empty(token_store_empty, approle, token):
+    """
+    Ensure a token is returned if no cached token is available
+    """
+    auth = vauth.VaultAppRoleAuth(approle, None, token_store=token_store_empty)
+    with patch("salt.utils.vault.auth.VaultAppRoleAuth._login") as login:
+        login.return_value = token
+        res = auth.get_token()
+        login.assert_called_once()
+        assert res == token
+
+
+def test_approle_auth_get_token_invalid(token_store_empty, approle_invalid):
+    """
+    Ensure VaultAuthExpired is raised if a token request was made, but
+    cannot be fulfilled
+    """
+    auth = vauth.VaultAppRoleAuth(approle_invalid, None, token_store=token_store_empty)
+    with pytest.raises(vault.VaultAuthExpired):
+        auth.get_token()
+
+
+@pytest.mark.parametrize("mount", ["approle", "salt_minions"])
+@pytest.mark.parametrize("approle", ["secret_id", None], indirect=True)
+def test_approle_auth_get_token_login(
+    approle, mount, client, token_store_empty_first, token
+):
+    """
+    Ensure that login with secret-id returns a token that is passed to the
+    token store/cache as well
+    """
+    auth = vauth.VaultAppRoleAuth(
+        approle, client, mount=mount, token_store=token_store_empty_first
+    )
+    res = auth.get_token()
+    assert res == token
+    args, kwargs = client.post.call_args
+    endpoint = args[0]
+    payload = kwargs.get("payload", {})
+    assert endpoint == f"auth/{mount}/login"
+    assert "role_id" in payload
+    if approle.secret_id is not None:
+        assert "secret_id" in payload
+    token_store_empty_first.replace_token.assert_called_once_with(res)
+
+
+@pytest.mark.parametrize("num_uses", [0, 1, 10])
+def test_approle_auth_used_num_uses(
+    token_store_empty_first, approle, client, uncached, num_uses, token
+):
+    """
+    Ensure that cache writes for use count are only done when
+    num_uses is not 0 (= unlimited)
+    """
+    approle.secret_id = approle.secret_id.with_renewed(num_uses=num_uses)
+    auth = vauth.VaultAppRoleAuth(
+        approle, client, cache=uncached, token_store=token_store_empty_first
+    )
+    res = auth.get_token()
+    assert res == token
+    if num_uses > 1:
+        uncached.store.assert_called_once_with(approle.secret_id)
+    elif num_uses:
+        uncached.store.assert_not_called()
+        uncached.flush.assert_called_once()
+    else:
+        uncached.store.assert_not_called()
+
+
+def test_approle_auth_used_locally_configured(
+    token_store_empty_first, approle, client, uncached, token
+):
+    """
+    Ensure that locally configured secret IDs are not cached.
+    """
+    approle.secret_id = vault.LocalVaultSecretId(**approle.secret_id.to_dict())
+    auth = vauth.VaultAppRoleAuth(
+        approle, client, cache=uncached, token_store=token_store_empty_first
+    )
+    res = auth.get_token()
+    assert res == token
+    uncached.store.assert_not_called()
+
+
+def test_approle_allows_no_secret_id():
+    """
+    Ensure AppRole containers are still valid if no
+    secret ID has been set (bind_secret_id can be set to False!)
+    """
+    role = vauth.VaultAppRole("test-role-id")
+    assert role.is_valid()

tests/pytests/unit/utils/vault/test_cache.py

@@ -0,0 +1,588 @@
+import copy
+import time
+
+import pytest
+
+import salt.cache
+import salt.utils.vault as vault
+import salt.utils.vault.cache as vcache
+from tests.support.mock import ANY, Mock, patch
+
+
+@pytest.fixture
+def cbank():
+    return "vault/connection"
+
+
+@pytest.fixture
+def ckey():
+    return "test"
+
+
+@pytest.fixture
+def data():
+    return {"foo": "bar"}
+
+
+@pytest.fixture
+def context(cbank, ckey, data):
+    return {cbank: {ckey: data}}
+
+
+@pytest.fixture
+def cached(cache_factory, data):
+    cache = Mock(spec=salt.cache.Cache)
+    cache.contains.return_value = True
+    cache.fetch.return_value = data
+    cache.updated.return_value = time.time()
+    cache_factory.return_value = cache
+    return cache
+
+
+@pytest.fixture
+def cached_outdated(cache_factory, data):
+    cache = Mock(spec=salt.cache.Cache)
+    cache.contains.return_value = True
+    cache.fetch.return_value = data
+    cache.updated.return_value = time.time() - 9999999
+    cache_factory.return_value = cache
+    return cache
+
+
+@pytest.fixture
+def uncached(cache_factory):
+    cache = Mock(spec=salt.cache.Cache)
+    cache.contains.return_value = False
+    cache.fetch.return_value = None
+    cache.updated.return_value = None
+    cache_factory.return_value = cache
+    return cache
+
+
+@pytest.fixture(autouse=True, params=[0])
+def time_stopped(request):
+    with patch(
+        "salt.utils.vault.cache.time.time", autospec=True, return_value=request.param
+    ):
+        yield
+
+
+@pytest.mark.parametrize("connection", [True, False])
+@pytest.mark.parametrize(
+    "salt_runtype,force_local,expected",
+    [
+        ("MASTER", False, "vault"),
+        ("MASTER_IMPERSONATING", False, "minions/test-minion/vault"),
+        ("MASTER_IMPERSONATING", True, "vault"),
+        ("MINION_LOCAL", False, "vault"),
+        ("MINION_REMOTE", False, "vault"),
+    ],
+    indirect=["salt_runtype"],
+)
+def test_get_cache_bank(connection, salt_runtype, force_local, expected):
+    """
+    Ensure the cache banks are mapped as expected, depending on run type
+    """
+    opts = {"grains": {"id": "test-minion"}}
+    cbank = vcache._get_cache_bank(opts, force_local=force_local, connection=connection)
+    if connection:
+        expected += "/connection"
+    assert cbank == expected
+
+
+class TestVaultCache:
+    @pytest.mark.parametrize("config", ["session", "other"])
+    def test_get_uncached(self, config, uncached, cbank, ckey):
+        """
+        Ensure that unavailable cached data is reported as None.
+        """
+        cache = vcache.VaultCache(
+            {}, cbank, ckey, cache_backend=uncached if config != "session" else None
+        )
+        res = cache.get()
+        assert res is None
+        if config != "session":
+            uncached.contains.assert_called_once_with(cbank, ckey)
+
+    def test_get_cached_from_context(self, context, cached, cbank, ckey, data):
+        """
+        Ensure that cached data in __context__ is respected, regardless
+        of cache backend.
+        """
+        cache = vcache.VaultCache(context, cbank, ckey, cache_backend=cached)
+        res = cache.get()
+        assert res == data
+        cached.updated.assert_not_called()
+        cached.fetch.assert_not_called()
+
+    def test_get_cached_not_outdated(self, cached, cbank, ckey, data):
+        """
+        Ensure that cached data that is still valid is returned.
+        """
+        cache = vcache.VaultCache({}, cbank, ckey, cache_backend=cached, ttl=3600)
+        res = cache.get()
+        assert res == data
+        cached.updated.assert_called_once_with(cbank, ckey)
+        cached.fetch.assert_called_once_with(cbank, ckey)
+
+    def test_get_cached_outdated(self, cached_outdated, cbank, ckey):
+        """
+        Ensure that cached data that is not valid anymore is flushed
+        and None is returned by default.
+        """
+        cache = vcache.VaultCache({}, cbank, ckey, cache_backend=cached_outdated, ttl=1)
+        res = cache.get()
+        assert res is None
+        cached_outdated.updated.assert_called_once_with(cbank, ckey)
+        cached_outdated.flush.assert_called_once_with(cbank, ckey)
+        cached_outdated.fetch.assert_not_called()
+
+    @pytest.mark.parametrize("config", ["session", "other"])
+    def test_flush(self, config, context, cached, cbank, ckey):
+        """
+        Ensure that flushing clears the context key only and, if
+        a cache backend is in use, it is also cleared.
+        """
+        cache = vcache.VaultCache(
+            context, cbank, ckey, cache_backend=cached if config != "session" else None
+        )
+        cache.flush()
+        assert context == {cbank: {}}
+        if config != "session":
+            cached.flush.assert_called_once_with(cbank, ckey)
+
+    @pytest.mark.parametrize("config", ["session", "other"])
+    def test_flush_cbank(self, config, context, cached, cbank, ckey):
+        """
+        Ensure that flushing with cbank=True clears the context bank and, if
+        a cache backend is in use, it is also cleared.
+        """
+        cache = vcache.VaultCache(
+            context, cbank, ckey, cache_backend=cached if config != "session" else None
+        )
+        cache.flush(cbank=True)
+        assert context == {}
+        if config != "session":
+            cached.flush.assert_called_once_with(cbank, None)
+
+    @pytest.mark.parametrize("context", [{}, {"vault/connection": {}}])
+    @pytest.mark.parametrize("config", ["session", "other"])
+    def test_store(self, config, context, uncached, cbank, ckey, data):
+        """
+        Ensure that storing data in cache always updates the context
+        and, if a cache backend is in use, it is also stored there.
+        """
+        cache = vcache.VaultCache(
+            context,
+            cbank,
+            ckey,
+            cache_backend=uncached if config != "session" else None,
+        )
+        cache.store(data)
+        assert context == {cbank: {ckey: data}}
+        if config != "session":
+            uncached.store.assert_called_once_with(cbank, ckey, data)
+
+
+class TestVaultConfigCache:
+    @pytest.fixture(params=["session", "other", None])
+    def config(self, request):
+        if request.param is None:
+            return None
+        return {
+            "cache": {
+                "backend": request.param,
+                "config": 3600,
+                "secret": "ttl",
+            }
+        }
+
+    @pytest.fixture
+    def data(self, config):
+        return {
+            "cache": {
+                "backend": "new",
+                "config": 1337,
+                "secret": "ttl",
+            }
+        }
+
+    @pytest.mark.usefixtures("uncached")
+    def test_get_config_cache_uncached(self, cbank, ckey):
+        """
+        Ensure an uninitialized instance is returned when there is no cache
+        """
+        res = vault.cache._get_config_cache({}, {}, cbank, ckey)
+        assert res.config is None
+
+    def test_get_config_context_cached(self, uncached, cbank, ckey, context):
+        """
+        Ensure cached data in context wins
+        """
+        res = vault.cache._get_config_cache({}, context, cbank, ckey)
+        assert res.config == context[cbank][ckey]
+        uncached.contains.assert_not_called()
+
+    def test_get_config_other_cached(self, cached, cbank, ckey, data):
+        """
+        Ensure cached data from other sources is respected
+        """
+        res = vault.cache._get_config_cache({}, {}, cbank, ckey)
+        assert res.config == data
+        cached.contains.assert_called_once_with(cbank, ckey)
+        cached.fetch.assert_called_once_with(cbank, ckey)
+
+    def test_reload(self, config, data, cbank, ckey):
+        """
+        Ensure that a changed configuration is reloaded correctly and
+        during instantiation. When the config backend changes and the
+        previous was not session only, it should be flushed.
+        """
+        with patch("salt.utils.vault.cache.VaultConfigCache.flush") as flush:
+            cache = vcache.VaultConfigCache({}, cbank, ckey, {}, init_config=config)
+            assert cache.config == config
+            if config is not None:
+                assert cache.ttl == config["cache"]["config"]
+                if config["cache"]["backend"] != "session":
+                    assert cache.cache is not None
+            else:
+                assert cache.ttl is None
+                assert cache.cache is None
+            cache._load(data)
+            assert cache.ttl == data["cache"]["config"]
+            assert cache.cache is not None
+            if config is not None and config["cache"]["backend"] != "session":
+                flush.assert_called_once()
+
+    @pytest.mark.usefixtures("cached")
+    def test_exists(self, config, context, cbank, ckey):
+        """
+        Ensure exists always evaluates to false when uninitialized
+        """
+        cache = vcache.VaultConfigCache(context, cbank, ckey, {}, init_config=config)
+        res = cache.exists()
+        assert res is bool(config)
+
+    def test_get(self, config, cached, context, cbank, ckey, data):
+        """
+        Ensure cached data is returned and backend settings honored,
+        unless the instance has not been initialized yet
+        """
+        if config is not None and config["cache"]["backend"] != "session":
+            context = {}
+        cache = vcache.VaultConfigCache(context, cbank, ckey, {}, init_config=config)
+        res = cache.get()
+        if config is not None:
+            assert res == data
+            if config["cache"]["backend"] != "session":
+                cached.fetch.assert_called_once_with(cbank, ckey)
+            else:
+                cached.contains.assert_not_called()
+                cached.fetch.assert_not_called()
+        else:
+            # uninitialized should always return None
+            # initialization when first stored or constructed with init_config
+            cached.contains.assert_not_called()
+            assert res is None
+
+    def test_flush(self, config, context, cached, cbank, ckey):
+        """
+        Ensure flushing deletes the whole cache bank (=connection scope),
+        unless the configuration has not been initialized.
+        Also, it should uninitialize the instance.
+        """
+        if config is None:
+            context_old = copy.deepcopy(context)
+        cache = vcache.VaultConfigCache(context, cbank, ckey, {}, init_config=config)
+        cache.flush()
+        if config is None:
+            assert context == context_old
+            cached.flush.assert_not_called()
+        else:
+            if config["cache"]["backend"] == "session":
+                assert context == {}
+            else:
+                cached.flush.assert_called_once_with(cbank, None)
+            assert cache.ttl is None
+            assert cache.cache is None
+            assert cache.config is None
+
+    @pytest.mark.usefixtures("uncached")
+    def test_store(self, data, cbank, ckey):
+        """
+        Ensure storing config in cache also reloads the instance
+        """
+        cache = vcache.VaultConfigCache({}, {}, cbank, ckey)
+        assert cache.config is None
+        with patch("salt.utils.vault.cache.VaultConfigCache._load") as rld:
+            with patch("salt.utils.vault.cache.VaultCache.store") as store:
+                cache.store(data)
+                rld.assert_called_once_with(data)
+                store.assert_called_once()
+
+    @pytest.mark.parametrize("config", ["other"], indirect=True)
+    def test_flush_exceptions_with_flush(self, config, cached, cbank, ckey):
+        """
+        Ensure internal flushing is disabled when the object is initialized
+        with a reference to an exception class.
+        """
+        cache = vcache.VaultConfigCache(
+            {},
+            cbank,
+            ckey,
+            {},
+            cache_backend_factory=lambda *args: cached,
+            flush_exception=vault.VaultConfigExpired,
+            init_config=config,
+        )
+        with pytest.raises(vault.VaultConfigExpired):
+            cache.flush()
+
+    @pytest.mark.parametrize("config", ["other"], indirect=True)
+    def test_flush_exceptions_with_get(self, config, cached_outdated, cbank, ckey):
+        """
+        Ensure internal flushing is disabled when the object is initialized
+        with a reference to an exception class.
+        """
+        cache = vcache.VaultConfigCache(
+            {},
+            cbank,
+            ckey,
+            {},
+            cache_backend_factory=lambda *args: cached_outdated,
+            flush_exception=vault.VaultConfigExpired,
+            init_config=config,
+        )
+        with pytest.raises(vault.VaultConfigExpired):
+            cache.get()
+
+
+class TestVaultAuthCache:
+    @pytest.fixture
+    def uncached(self):
+        with patch(
+            "salt.utils.vault.cache.CommonCache._ckey_exists",
+            return_value=False,
+            autospec=True,
+        ):
+            with patch(
+                "salt.utils.vault.cache.CommonCache._get_ckey",
+                return_value=None,
+                autospec=True,
+            ) as get:
+                yield get
+
+    @pytest.fixture
+    def cached(self, token_auth):
+        with patch(
+            "salt.utils.vault.cache.CommonCache._ckey_exists",
+            return_value=True,
+            autospec=True,
+        ):
+            with patch(
+                "salt.utils.vault.cache.CommonCache._get_ckey",
+                return_value=token_auth["auth"],
+                autospec=True,
+            ) as get:
+                yield get
+
+    @pytest.fixture
+    def cached_outdated(self, token_auth):
+        with patch(
+            "salt.utils.vault.cache.CommonCache._ckey_exists",
+            return_value=True,
+            autospec=True,
+        ):
+            token_auth["auth"]["creation_time"] = 0
+            token_auth["auth"]["lease_duration"] = 1
+            with patch(
+                "salt.utils.vault.cache.CommonCache._get_ckey",
+                return_value=token_auth["auth"],
+                autospec=True,
+            ) as get:
+                yield get
+
+    @pytest.fixture
+    def cached_invalid_flush(self, token_auth, cached):
+        with patch("salt.utils.vault.cache.CommonCache._flush", autospec=True) as flush:
+            token_auth["auth"]["num_uses"] = 1
+            token_auth["auth"]["use_count"] = 1
+            cached.return_value = token_auth["auth"]
+            yield flush
+
+    @pytest.mark.usefixtures("uncached")
+    def test_get_uncached(self):
+        """
+        Ensure that unavailable cached data is reported as None.
+        """
+        cache = vcache.VaultAuthCache({}, "cbank", "ckey", vault.VaultToken)
+        res = cache.get()
+        assert res is None
+
+    @pytest.mark.usefixtures("cached")
+    def test_get_cached(self, token_auth):
+        """
+        Ensure that cached data that is still valid is returned.
+        """
+        cache = vcache.VaultAuthCache({}, "cbank", "ckey", vault.VaultToken)
+        res = cache.get()
+        assert res is not None
+        assert res == vault.VaultToken(**token_auth["auth"])
+
+    def test_get_cached_invalid(self, cached_invalid_flush):
+        """
+        Ensure that cached data that is not valid anymore is flushed
+        and None is returned.
+        """
+        cache = vcache.VaultAuthCache({}, "cbank", "ckey", vault.VaultToken)
+        res = cache.get()
+        assert res is None
+        cached_invalid_flush.assert_called_once()
+
+    def test_store(self, token_auth):
+        """
+        Ensure that storing authentication data sends a dictionary
+        representation to the store implementation of the parent class.
+        """
+        token = vault.VaultToken(**token_auth["auth"])
+        cache = vcache.VaultAuthCache({}, "cbank", "ckey", vault.VaultToken)
+        with patch("salt.utils.vault.cache.CommonCache._store_ckey") as store:
+            cache.store(token)
+            store.assert_called_once_with("ckey", token.to_dict())
+
+    def test_flush_exceptions_with_flush(self, cached, cbank, ckey):
+        """
+        Ensure internal flushing is disabled when the object is initialized
+        with a reference to an exception class.
+        """
+        cache = vcache.VaultAuthCache(
+            {},
+            cbank,
+            ckey,
+            vault.VaultToken,
+            cache_backend=cached,
+            flush_exception=vault.VaultAuthExpired,
+        )
+        with pytest.raises(vault.VaultAuthExpired):
+            cache.flush()
+
+    def test_flush_exceptions_with_get(self, cached_outdated, cbank, ckey):
+        """
+        Ensure internal flushing is disabled when the object is initialized
+        with a reference to an exception class.
+        """
+        cache = vcache.VaultAuthCache(
+            {}, cbank, ckey, vault.VaultToken, flush_exception=vault.VaultAuthExpired
+        )
+        with pytest.raises(vault.VaultAuthExpired):
+            cache.get(10)
+
+
+class TestVaultLeaseCache:
+    @pytest.fixture
+    def uncached(self):
+        with patch(
+            "salt.utils.vault.cache.CommonCache._ckey_exists",
+            return_value=False,
+            autospec=True,
+        ):
+            with patch(
+                "salt.utils.vault.cache.CommonCache._get_ckey",
+                return_value=None,
+                autospec=True,
+            ) as get:
+                yield get
+
+    @pytest.fixture
+    def cached(self, lease):
+        with patch(
+            "salt.utils.vault.cache.CommonCache._ckey_exists",
+            return_value=True,
+            autospec=True,
+        ):
+            with patch(
+                "salt.utils.vault.cache.CommonCache._get_ckey",
+                return_value=lease,
+                autospec=True,
+            ) as get:
+                yield get
+
+    @pytest.fixture
+    def cached_outdated(self, lease):
+        with patch(
+            "salt.utils.vault.cache.CommonCache._ckey_exists",
+            return_value=True,
+            autospec=True,
+        ):
+            lease["duration"] = 6
+            lease["expire_time"] = 6
+            with patch(
+                "salt.utils.vault.cache.CommonCache._get_ckey",
+                return_value=lease,
+                autospec=True,
+            ) as get:
+                yield get
+
+    @pytest.mark.usefixtures("uncached")
+    def test_get_uncached(self):
+        """
+        Ensure that unavailable cached data is reported as None.
+        """
+        cache = vcache.VaultLeaseCache({}, "cbank")
+        res = cache.get("testlease")
+        assert res is None
+
+    @pytest.mark.usefixtures("cached")
+    def test_get_cached(self, lease):
+        """
+        Ensure that cached data that is still valid is returned.
+        """
+        cache = vcache.VaultLeaseCache({}, "cbank")
+        res = cache.get("testlease")
+        assert res is not None
+        assert res == vault.VaultLease(**lease)
+
+    @pytest.mark.usefixtures("cached", "time_stopped")
+    @pytest.mark.parametrize("valid_for,expected", ((1, True), (99999999, False)))
+    def test_get_cached_valid_for(self, valid_for, expected, lease):
+        """
+        Ensure that requesting leases with a validity works as expected.
+        The lease should be returned if it is valid, otherwise only
+        the invalid ckey should be flushed and None returned.
+        """
+        cache = vcache.VaultLeaseCache({}, "cbank")
+        with patch(
+            "salt.utils.vault.cache.CommonCache._flush",
+            autospec=True,
+        ) as flush:
+            res = cache.get("testlease", valid_for=valid_for, flush=True)
+            if expected:
+                flush.assert_not_called()
+                assert res is not None
+                assert res == vault.VaultLease(**lease)
+            else:
+                flush.assert_called_once_with(ANY, "testlease")
+                assert res is None
+
+    def test_store(self, lease):
+        """
+        Ensure that storing authentication data sends a dictionary
+        representation to the store implementation of the parent class.
+        """
+        lease_ = vault.VaultLease(**lease)
+        cache = vcache.VaultLeaseCache({}, "cbank")
+        with patch("salt.utils.vault.cache.CommonCache._store_ckey") as store:
+            cache.store("ckey", lease_)
+            store.assert_called_once_with("ckey", lease_.to_dict())
+
+    def test_expire_events_with_get(self, events, cached_outdated, cbank, ckey, lease):
+        """
+        Ensure internal flushing is disabled when the object is initialized
+        with a reference to an exception class.
+        """
+        cache = vcache.VaultLeaseCache({}, "cbank", expire_events=events)
+        ret = cache.get("ckey", 10)
+        assert ret is None
+        events.assert_called_once_with(
+            tag="vault/lease/ckey/expire", data={"valid_for_less": 10}
+        )

tests/pytests/unit/utils/vault/test_client.py

@@ -0,0 +1,650 @@
+import pytest
+import requests
+
+import salt.exceptions
+import salt.utils.vault as vault
+import salt.utils.vault.client as vclient
+from tests.pytests.unit.utils.vault.conftest import _mock_json_response
+from tests.support.mock import ANY, Mock, patch
+
+
+@pytest.mark.parametrize(
+    "endpoint",
+    [
+        "secret/some/path",
+        "/secret/some/path",
+        "secret/some/path/",
+        "/secret/some/path/",
+    ],
+)
+def test_vault_client_request_raw_url(endpoint, client, req):
+    """
+    Test that requests are sent to the correct endpoint, regardless of leading or trailing slashes
+    """
+    expected_url = f"{client.url}/v1/secret/some/path"
+    client.request_raw("GET", endpoint)
+    req.assert_called_with(
+        "GET",
+        expected_url,
+        headers=ANY,
+        json=None,
+        verify=client.get_config()["verify"],
+    )
+
+
+def test_vault_client_request_raw_kwargs_passthrough(client, req):
+    """
+    Test that kwargs for requests.request are passed through
+    """
+    client.request_raw(
+        "GET", "secret/some/path", allow_redirects=False, cert="/etc/certs/client.pem"
+    )
+    req.assert_called_with(
+        "GET",
+        ANY,
+        headers=ANY,
+        json=ANY,
+        verify=ANY,
+        allow_redirects=False,
+        cert="/etc/certs/client.pem",
+    )
+
+
+@pytest.mark.parametrize("namespace", [None, "test-namespace"])
+@pytest.mark.parametrize("client", [None], indirect=True)
+def test_vault_client_request_raw_headers_namespace(namespace, client, req):
+    """
+    Test that namespace is present in the HTTP headers only if it was specified
+    """
+    if namespace is not None:
+        client.namespace = namespace
+
+    namespace_header = "X-Vault-Namespace"
+    client.request_raw("GET", "secret/some/path")
+    headers = req.call_args.kwargs.get("headers", {})
+    if namespace is None:
+        assert namespace_header not in headers
+    else:
+        assert headers.get(namespace_header) == namespace
+
+
+@pytest.mark.parametrize("wrap", [False, 30, "1h"])
+def test_vault_client_request_raw_headers_wrap(wrap, client, req):
+    """
+    Test that the wrap header is present only if it was specified and supports time strings
+    """
+    wrap_header = "X-Vault-Wrap-TTL"
+    client.request_raw("GET", "secret/some/path", wrap=wrap)
+    headers = req.call_args.kwargs.get("headers", {})
+    if not wrap:
+        assert wrap_header not in headers
+    else:
+        assert headers.get(wrap_header) == str(wrap)
+
+
+@pytest.mark.parametrize("header", ["X-Custom-Header", "X-Existing-Header"])
+def test_vault_client_request_raw_headers_additional(header, client, req):
+    """
+    Test that additional headers are passed correctly and override default ones
+    """
+    with patch.object(
+        client, "_get_headers", Mock(return_value={"X-Existing-Header": "unchanged"})
+    ):
+        client.request_raw("GET", "secret/some/path", add_headers={header: "changed"})
+        actual_header = req.call_args.kwargs.get("headers", {}).get(header)
+        assert actual_header == "changed"
+
+
+@pytest.mark.usefixtures("req_failed")
+@pytest.mark.parametrize(
+    "req_failed",
+    [400, 403, 404, 502, 401],
+    indirect=True,
+)
+@pytest.mark.parametrize(
+    "client",
+    [None],
+    indirect=True,
+)
+def test_vault_client_request_raw_does_not_raise_http_exception(client):
+    """
+    request_raw should return the raw response object regardless of HTTP status code
+    """
+    res = client.request_raw("GET", "secret/some/path")
+    with pytest.raises(requests.exceptions.HTTPError):
+        res.raise_for_status()
+
+
+@pytest.mark.parametrize(
+    "req_failed,expected",
+    [
+        (400, vault.VaultInvocationError),
+        (403, vault.VaultPermissionDeniedError),
+        (404, vault.VaultNotFoundError),
+        (405, vault.VaultUnsupportedOperationError),
+        (412, vault.VaultPreconditionFailedError),
+        (500, vault.VaultServerError),
+        (502, vault.VaultServerError),
+        (503, vault.VaultUnavailableError),
+        (401, requests.exceptions.HTTPError),
+    ],
+    indirect=["req_failed"],
+)
+@pytest.mark.parametrize("raise_error", [True, False])
+def test_vault_client_request_respects_raise_error(
+    raise_error, req_failed, expected, client
+):
+    """
+    request should inspect the response object and raise appropriate errors
+    or fall back to raise_for_status if raise_error is true
+    """
+    if raise_error:
+        with pytest.raises(expected):
+            client.request("GET", "secret/some/path", raise_error=raise_error)
+    else:
+        res = client.request("GET", "secret/some/path", raise_error=raise_error)
+        assert "errors" in res
+
+
+def test_vault_client_request_returns_whole_response_data(
+    role_id_response, req, client
+):
+    """
+    request should return the whole returned payload, not auth/data etc only
+    """
+    req.return_value = _mock_json_response(role_id_response)
+    res = client.request("GET", "auth/approle/role/test-minion/role-id")
+    assert res == role_id_response
+
+
+def test_vault_client_request_hydrates_wrapped_response(
+    wrapped_role_id_response, req, client
+):
+    """
+    request should detect wrapped responses and return an instance of VaultWrappedResponse
+    instead of raw data
+    """
+    req.return_value = _mock_json_response(wrapped_role_id_response)
+    res = client.request("GET", "auth/approle/role/test-minion/role-id", wrap="180s")
+    assert isinstance(res, vault.VaultWrappedResponse)
+
+
+@pytest.mark.usefixtures("req_success")
+def test_vault_client_request_returns_true_when_no_data_is_reported(client):
+    """
+    HTTP 204 indicates success with no data returned
+    """
+    res = client.request("GET", "secret/some/path")
+    assert res is True
+
+
+def test_vault_client_get_config(server_config, client):
+    """
+    The returned configuration should match the one used to create an instance of VaultClient
+    """
+    assert client.get_config() == server_config
+
+
+@pytest.mark.parametrize("client", [None], indirect=["client"])
+def test_vault_client_token_valid_false(client):
+    """
+    The unauthenticated client should always report the token as being invalid
+    """
+    assert client.token_valid() is False
+
+
+@pytest.mark.parametrize("client", ["valid_token", "invalid_token"], indirect=True)
+@pytest.mark.parametrize("req_any", [200, 403], indirect=True)
+@pytest.mark.parametrize("remote", [False, True])
+def test_vault_client_token_valid(client, remote, req_any):
+    valid = client.token_valid(remote=remote)
+    if not remote or not client.auth.is_valid():
+        req_any.assert_not_called()
+    else:
+        req_any.assert_called_once()
+    should_be_valid = client.auth.is_valid() and (
+        not remote or req_any("POST", "abc").status_code == 200
+    )
+    assert valid is should_be_valid
+
+
+@pytest.mark.parametrize("func", ["get", "delete", "post", "list"])
+def test_vault_client_wrapper_should_not_require_payload(func, client, req):
+    """
+    Check that wrappers for get/delete/post/list do not require a payload
+    """
+    req.return_value = _mock_json_response({}, status_code=200)
+    tgt = getattr(client, func)
+    res = tgt("auth/approle/role/test-role/secret-id")
+    assert res == {}
+
+
+@pytest.mark.parametrize("func", ["patch"])
+def test_vault_client_wrapper_should_require_payload(func, client, req):
+    """
+    Check that patch wrapper does require a payload
+    """
+    req.return_value = _mock_json_response({}, status_code=200)
+    tgt = getattr(client, func)
+    with pytest.raises(TypeError):
+        tgt("auth/approle/role/test-role/secret-id")
+
+
+def test_vault_client_wrap_info_only_data(wrapped_role_id_lookup_response, client, req):
+    """
+    wrap_info should only return the data portion of the returned wrapping information
+    """
+    req.return_value = _mock_json_response(wrapped_role_id_lookup_response)
+    res = client.wrap_info("test-wrapping-token")
+    assert res == wrapped_role_id_lookup_response["data"]
+
+
+@pytest.mark.parametrize(
+    "req_failed,expected", [(502, vault.VaultServerError)], indirect=["req_failed"]
+)
+def test_vault_client_wrap_info_should_fail_with_sensible_response(
+    req_failed, expected, client
+):
+    """
+    wrap_info should return sensible Exceptions, not KeyError etc
+    """
+    with pytest.raises(expected):
+        client.wrap_info("test-wrapping-token")
+
+
+def test_vault_client_unwrap_returns_whole_response(role_id_response, client, req):
+    """
+    The unwrapped response should be returned as a whole, not auth/data etc only
+    """
+    req.return_value = _mock_json_response(role_id_response)
+    res = client.unwrap("test-wrapping-token")
+    assert res == role_id_response
+
+
+def test_vault_client_unwrap_should_default_to_token_header_before_payload(
+    role_id_response, client, req
+):
+    """
+    When unwrapping a wrapping token, it can be used as the authentication token header.
+    If the client has a valid token, it should be used in the header instead and the
+    unwrapping token should be passed in the payload
+    """
+    token = "test-wrapping-token"
+    req.return_value = _mock_json_response(role_id_response)
+    client.unwrap(token)
+    if client.token_valid(remote=False):
+        payload = req.call_args.kwargs.get("json", {})
+        assert payload.get("token") == token
+    else:
+        headers = req.call_args.kwargs.get("headers", {})
+        assert headers.get("X-Vault-Token") == token
+
+
+@pytest.mark.parametrize("func", ["unwrap", "token_lookup"])
+@pytest.mark.parametrize(
+    "req_failed,expected",
+    [
+        (400, vault.VaultInvocationError),
+        (403, vault.VaultPermissionDeniedError),
+        (404, vault.VaultNotFoundError),
+        (502, vault.VaultServerError),
+        (401, requests.exceptions.HTTPError),
+    ],
+    indirect=["req_failed"],
+)
+def test_vault_client_unwrap_should_raise_appropriate_errors(
+    func, req_failed, expected, client
+):
+    """
+    unwrap/token_lookup should raise exceptions the same way request does
+    """
+    with pytest.raises(expected):
+        tgt = getattr(client, func)
+        tgt("test-wrapping-token")
+
+
+@pytest.mark.usefixtures("req_unwrapping")
+@pytest.mark.parametrize(
+    "path",
+    [
+        "auth/approle/role/test-minion/role-id",
+        "auth/approle/role/[^/]+/role-id",
+        ["incorrect/path", "[^a]+", "auth/approle/role/[^/]+/role-id"],
+    ],
+)
+def test_vault_client_unwrap_should_match_check_expected_creation_path(
+    path, role_id_response, client
+):
+    """
+    Expected creation paths should be accepted as strings and list of strings,
+    where the strings can be regex patterns
+    """
+    res = client.unwrap("test-wrapping-token", expected_creation_path=path)
+    assert res == role_id_response
+
+
+@pytest.mark.usefixtures("req_unwrapping")
+@pytest.mark.parametrize(
+    "path",
+    [
+        "auth/other_mount/role/test-minion/role-id",
+        "auth/approle/role/[^tes/]+/role-id",
+        ["incorrect/path", "[^a]+", "auth/approle/role/[^/]/role-id"],
+    ],
+)
+def test_vault_client_unwrap_should_fail_on_unexpected_creation_path(path, client):
+    """
+    When none of the patterns match, a (serious) exception should be raised
+    """
+    with pytest.raises(vault.VaultUnwrapException):
+        client.unwrap("test-wrapping-token", expected_creation_path=path)
+
+
+def test_vault_client_token_lookup_returns_data_only(
+    token_lookup_self_response, req, client
+):
+    """
+    token_lookup should return "data" only, not the whole response payload
+    """
+    req.return_value = _mock_json_response(token_lookup_self_response)
+    res = client.token_lookup("test-token")
+    assert res == token_lookup_self_response["data"]
+
+
+@pytest.mark.parametrize("raw", [False, True])
+def test_vault_client_token_lookup_respects_raw(raw, req, client):
+    """
+    when raw is True, token_lookup should return the raw response
+    """
+    response_data = {"foo": "bar"}
+    req.return_value = _mock_json_response({"data": response_data})
+    res = client.token_lookup("test-token", raw=raw)
+    if raw:
+        assert res.json() == {"data": response_data}
+    else:
+        assert res == response_data
+
+
+def test_vault_client_token_lookup_uses_accessor(client, req_any):
+    """
+    Ensure a client can lookup tokens with provided accessor
+    """
+    token = "test-token"
+    if client.token_valid():
+        token = None
+    client.token_lookup(token=token, accessor="test-token-accessor")
+    payload = req_any.call_args.kwargs.get("json", {})
+    _, url = req_any.call_args[0]
+    assert payload.get("accessor") == "test-token-accessor"
+    assert url.endswith("lookup-accessor")
+
+
+# VaultClient only
+
+
+@pytest.mark.usefixtures("req")
+@pytest.mark.parametrize("client", [None], indirect=["client"])
+def test_vault_client_token_lookup_requires_token_for_unauthenticated_client(client):
+    with pytest.raises(vault.VaultInvocationError):
+        client.token_lookup()
+
+
+# AuthenticatedVaultClient only
+
+
+@pytest.mark.usefixtures("req_any")
+@pytest.mark.parametrize("client", ["valid_token"], indirect=True)
+@pytest.mark.parametrize(
+    "endpoint,use",
+    [
+        ("secret/data/some/path", True),
+        ("auth/approle/role/test-minion", True),
+        ("sys/internal/ui/mounts", False),
+        ("sys/internal/ui/mounts/secret", False),
+        ("sys/wrapping/lookup", False),
+        ("sys/internal/ui/namespaces", False),
+        ("sys/health", False),
+        ("sys/seal-status", False),
+    ],
+)
+def test_vault_client_request_raw_increases_use_count_when_necessary_depending_on_path(
+    endpoint, use, client
+):
+    """
+    When a request is issued to an endpoint that consumes a use, ensure it is passed
+    along to the token.
+    https://github.com/hashicorp/vault/blob/d467681e15898041b6dd5f2bf7789bd7c236fb16/vault/logical_system.go#L119-L155
+    """
+    client.request_raw("GET", endpoint)
+    assert client.auth.used.called is use
+
+
+@pytest.mark.parametrize("client", ["valid_token"], indirect=True)
+@pytest.mark.parametrize(
+    "req_failed",
+    [400, 403, 404, 405, 412, 500, 502, 503, 401],
+    indirect=True,
+)
+def test_vault_client_request_raw_increases_use_count_when_necessary_depending_on_response(
+    req_failed, client
+):
+    """
+    When a request is issued to an endpoint that consumes a use, make sure that
+    this is registered regardless of status code:
+    https://github.com/hashicorp/vault/blob/c1cf97adac5c53301727623a74b828a5f12592cf/vault/request_handling.go#L864-L866
+    ref: PR #62552
+    """
+    client.request_raw("GET", "secret/data/some/path")
+    assert client.auth.used.called is True
+
+
+@pytest.mark.usefixtures("req_any")
+@pytest.mark.parametrize("client", ["valid_token"], indirect=True)
+def test_vault_client_request_raw_does_not_increase_use_count_with_unauthd_endpoint(
+    client,
+):
+    """
+    Unauthenticated endpoints do not consume a token use. Since some cannot be detected
+    easily because of customizable mount points for secret engines and auth methods,
+    this can be specified in the request. Make sure it is honored.
+    """
+    client.request("GET", "pki/cert/ca", is_unauthd=True)
+    client.auth.used.assert_not_called()
+
+
+@pytest.mark.parametrize("client", ["valid_token"], indirect=True)
+def test_vault_client_token_lookup_self_possible(client, req_any):
+    """
+    Ensure an authenticated client can lookup its own token
+    """
+    client.token_lookup()
+    headers = req_any.call_args.kwargs.get("headers", {})
+    _, url = req_any.call_args[0]
+    assert headers.get("X-Vault-Token") == str(client.auth.get_token())
+    assert url.endswith("lookup-self")
+
+
+@pytest.mark.parametrize("client", ["valid_token"], indirect=True)
+def test_vault_client_token_lookup_supports_token_arg(client, req_any):
+    """
+    Ensure an authenticated client can lookup other tokens
+    """
+    token = "other-test-token"
+    client.token_lookup(token=token)
+    headers = req_any.call_args.kwargs.get("headers", {})
+    payload = req_any.call_args.kwargs.get("json", {})
+    _, url = req_any.call_args[0]
+    assert payload.get("token") == token
+    assert headers.get("X-Vault-Token") == str(client.auth.get_token())
+    assert url.endswith("lookup")
+
+
+@pytest.mark.parametrize("client", ["valid_token"], indirect=True)
+@pytest.mark.parametrize("renewable", [True, False])
+def test_vault_client_token_renew_self_possible(
+    token_renew_self_response, client, req, renewable
+):
+    """
+    Ensure an authenticated client can renew its own token only when
+    it is renewable and that the renewed data is passed along to the
+    token store
+    """
+    req.return_value = _mock_json_response(token_renew_self_response)
+    client.auth.is_renewable.return_value = renewable
+    res = client.token_renew()
+    if renewable:
+        headers = req.call_args.kwargs.get("headers", {})
+        _, url = req.call_args[0]
+        assert headers.get("X-Vault-Token") == str(client.auth.get_token())
+        assert url.endswith("renew-self")
+        req.assert_called_once()
+        client.auth.update_token.assert_called_once_with(
+            token_renew_self_response["auth"]
+        )
+        assert res == token_renew_self_response["auth"]
+    else:
+        assert res is False
+
+
+@pytest.mark.parametrize("client", ["valid_token"], indirect=True)
+def test_vault_client_token_renew_supports_token_arg(
+    token_renew_other_response, client, req
+):
+    """
+    Ensure an authenticated client can renew other tokens
+    """
+    req.return_value = _mock_json_response(token_renew_other_response)
+    token = "other-test-token"
+    client.token_renew(token=token)
+    headers = req.call_args.kwargs.get("headers", {})
+    payload = req.call_args.kwargs.get("json", {})
+    _, url = req.call_args[0]
+    assert payload.get("token") == token
+    assert headers.get("X-Vault-Token") == str(client.auth.get_token())
+    assert url.endswith("renew")
+
+
+@pytest.mark.parametrize("client", ["valid_token"], indirect=True)
+def test_vault_client_token_renew_uses_accessor(
+    token_renew_accessor_response, client, req
+):
+    """
+    Ensure a client can renew tokens with provided accessor
+    """
+    req.return_value = _mock_json_response(token_renew_accessor_response)
+    client.token_renew(accessor="test-token-accessor")
+    payload = req.call_args.kwargs.get("json", {})
+    _, url = req.call_args[0]
+    assert payload.get("accessor") == "test-token-accessor"
+    assert url.endswith("renew-accessor")
+
+
+@pytest.mark.parametrize("client", ["valid_token"], indirect=True)
+@pytest.mark.parametrize("token", [None, "other-test-token"])
+def test_vault_client_token_renew_self_updates_token(
+    token_renew_self_response, client, token, req
+):
+    """
+    Ensure the current client token is updated when it is renewed, but not
+    when another token is renewed
+    """
+    req.return_value = _mock_json_response(token_renew_self_response)
+    client.token_renew(token=token)
+    if token is None:
+        assert client.auth.update_token.called
+    else:
+        assert not client.auth.update_token.called
+
+
+@pytest.mark.parametrize("client", ["valid_token"], indirect=True)
+@pytest.mark.parametrize(
+    "token,accessor",
+    [(None, None), ("other-test-token", None), (None, "test-accessor")],
+)
+def test_vault_client_token_renew_increment_is_honored(
+    token, accessor, client, token_renew_self_response, req
+):
+    """
+    Ensure the renew increment is passed to vault if provided
+    """
+    req.return_value = _mock_json_response(token_renew_self_response)
+    client.token_renew(token=token, accessor=accessor, increment=3600)
+    payload = req.call_args.kwargs.get("json", {})
+    assert payload.get("increment") == 3600
+
+
+@pytest.mark.parametrize(
+    "secret,config,expected",
+    [
+        ("token", None, r"auth/token/create(/[^/]+)?"),
+        ("secret_id", None, r"auth/[^/]+/role/[^/]+/secret\-id"),
+        ("role_id", None, r"auth/[^/]+/role/[^/]+/role\-id"),
+        (
+            "secret_id",
+            {"auth": {"approle_mount": "test_mount", "approle_name": "test_minion"}},
+            r"auth/test_mount/role/test_minion/secret\-id",
+        ),
+        (
+            "role_id",
+            {"auth": {"approle_mount": "test_mount", "approle_name": "test_minion"}},
+            r"auth/test_mount/role/test_minion/role\-id",
+        ),
+        (
+            "secret_id",
+            {"auth": {"approle_mount": "te$t-mount", "approle_name": "te$t-minion"}},
+            r"auth/te\$t\-mount/role/te\$t\-minion/secret\-id",
+        ),
+        (
+            "role_id",
+            {"auth": {"approle_mount": "te$t-mount", "approle_name": "te$t-minion"}},
+            r"auth/te\$t\-mount/role/te\$t\-minion/role\-id",
+        ),
+    ],
+)
+def test_get_expected_creation_path(secret, config, expected):
+    """
+    Ensure expected creation paths are resolved as expected
+    """
+    assert vclient._get_expected_creation_path(secret, config) == expected
+
+
+def test_get_expected_creation_path_fails_for_unknown_type():
+    """
+    Ensure unknown source types result in an exception
+    """
+    with pytest.raises(salt.exceptions.SaltInvocationError):
+        vclient._get_expected_creation_path("nonexistent")
+
+
+@pytest.mark.parametrize(
+    "server_config",
+    [
+        {
+            "url": "https://127.0.0.1:8200",
+            "verify": "-----BEGIN CERTIFICATE-----testcert",
+        }
+    ],
+    indirect=True,
+)
+def test_vault_client_verify_pem(server_config):
+    """
+    Test that the ``verify`` parameter to the client can contain a PEM-encoded certificate
+    which will be used as the sole trust anchor for the Vault URL.
+    The ``verify`` parameter to ``Session.request`` should be None in that case since
+    it requires a local file path.
+    """
+    with patch("salt.utils.vault.client.CACertHTTPSAdapter", autospec=True) as adapter:
+        with patch("salt.utils.vault.requests.Session", autospec=True) as session:
+            client = vclient.VaultClient(**server_config)
+            adapter.assert_called_once_with(server_config["verify"])
+            session.return_value.mount.assert_called_once_with(
+                server_config["url"], adapter.return_value
+            )
+            client.request_raw("GET", "test")
+            session.return_value.request.assert_called_once_with(
+                "GET",
+                f"{server_config['url']}/v1/test",
+                headers=ANY,
+                json=ANY,
+                verify=None,
+            )

tests/pytests/unit/utils/vault/test_helpers.py

@@ -0,0 +1,119 @@
+# this needs to be from! see test_iso_to_timestamp_polyfill
+from datetime import datetime
+
+import pytest
+
+import salt.utils.vault.helpers as hlp
+from tests.support.mock import patch
+
+
+@pytest.mark.parametrize(
+    "opts_runtype,expected",
+    [
+        ("master", hlp.SALT_RUNTYPE_MASTER),
+        ("master_peer_run", hlp.SALT_RUNTYPE_MASTER_PEER_RUN),
+        ("master_impersonating", hlp.SALT_RUNTYPE_MASTER_IMPERSONATING),
+        ("minion_local_1", hlp.SALT_RUNTYPE_MINION_LOCAL),
+        ("minion_local_2", hlp.SALT_RUNTYPE_MINION_LOCAL),
+        ("minion_local_3", hlp.SALT_RUNTYPE_MINION_LOCAL),
+        ("minion_remote", hlp.SALT_RUNTYPE_MINION_REMOTE),
+    ],
+    indirect=["opts_runtype"],
+)
+def test_get_salt_run_type(opts_runtype, expected):
+    """
+    Ensure run types are detected as expected
+    """
+    assert hlp._get_salt_run_type(opts_runtype) == expected
+
+
+@pytest.mark.parametrize(
+    "pattern,expected",
+    [
+        ("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",
+            ],
+        ),
+    ],
+)
+def test_expand_pattern_lists(pattern, expected):
+    """
+    Ensure expand_pattern_lists works as intended:
+    - Expand list-valued patterns
+    - Do not change non-list-valued tokens
+    """
+    pattern_vars = {
+        "id": "test-minion",
+        "roles": ["web", "database"],
+        "aux": ["foo", "bar"],
+        "deep": {"foo": {"bar": {"baz": ["hello", "world"]}}},
+    }
+
+    mappings = {"minion": "test-minion", "grains": pattern_vars}
+    output = hlp.expand_pattern_lists(pattern, **mappings)
+    assert output == expected
+
+
+@pytest.mark.parametrize(
+    "inpt,expected",
+    [
+        (60.0, 60.0),
+        (60, 60.0),
+        ("60", 60.0),
+        ("60s", 60.0),
+        ("2m", 120.0),
+        ("1h", 3600.0),
+        ("1d", 86400.0),
+        ("1.5s", 1.5),
+        ("1.5m", 90.0),
+        ("1.5h", 5400.0),
+        ("7.5d", 648000.0),
+    ],
+)
+def test_timestring_map(inpt, expected):
+    assert hlp.timestring_map(inpt) == expected
+
+
+@pytest.mark.parametrize(
+    "creation_time,expected",
+    [
+        ("2022-08-22T17:16:21-09:30", 1661222781),
+        ("2022-08-22T17:16:21-01:00", 1661192181),
+        ("2022-08-22T17:16:21+00:00", 1661188581),
+        ("2022-08-22T17:16:21Z", 1661188581),
+        ("2022-08-22T17:16:21+02:00", 1661181381),
+        ("2022-08-22T17:16:21+12:30", 1661143581),
+    ],
+)
+def test_iso_to_timestamp_polyfill(creation_time, expected):
+    with patch("salt.utils.vault.helpers.datetime.datetime") as d:
+        d.fromisoformat.side_effect = AttributeError
+        # needs from datetime import datetime, otherwise results
+        # in infinite recursion
+
+        # pylint: disable=unnecessary-lambda
+        d.side_effect = lambda *args: datetime(*args)
+        res = hlp.iso_to_timestamp(creation_time)
+        assert res == expected

tests/pytests/unit/utils/vault/test_kv.py

@@ -0,0 +1,592 @@
+import pytest
+import requests.models
+
+import salt.utils.vault as vault
+import salt.utils.vault.cache as vcache
+import salt.utils.vault.client as vclient
+import salt.utils.vault.kv as vkv
+from tests.support.mock import MagicMock, Mock, patch
+
+
+@pytest.fixture
+def path():
+    return "secret/some/path"
+
+
+@pytest.fixture
+def paths():
+    return {
+        "data": "secret/data/some/path",
+        "metadata": "secret/metadata/some/path",
+        "delete": "secret/data/some/path",
+        "delete_versions": "secret/delete/some/path",
+        "destroy": "secret/destroy/some/path",
+    }
+
+
+@pytest.fixture
+def kvv1_meta_response():
+    return {
+        "request_id": "b82f2df7-a9b6-920c-0ed2-a3463b996f9e",
+        "lease_id": "",
+        "renewable": False,
+        "lease_duration": 0,
+        "data": {
+            "accessor": "kv_f8731f1b",
+            "config": {
+                "default_lease_ttl": 0,
+                "force_no_cache": False,
+                "max_lease_ttl": 0,
+            },
+            "description": "key/value secret storage",
+            "external_entropy_access": False,
+            "local": False,
+            "options": None,
+            "path": "secret/",
+            "seal_wrap": False,
+            "type": "kv",
+            "uuid": "1d9431ac-060a-9b63-4572-3ca7ffd78347",
+        },
+        "wrap_info": None,
+        "warnings": None,
+        "auth": None,
+    }
+
+
+@pytest.fixture
+def kvv2_meta_response():
+    return {
+        "request_id": "b82f2df7-a9b6-920c-0ed2-a3463b996f9e",
+        "lease_id": "",
+        "renewable": False,
+        "lease_duration": 0,
+        "data": {
+            "accessor": "kv_f8731f1b",
+            "config": {
+                "default_lease_ttl": 0,
+                "force_no_cache": False,
+                "max_lease_ttl": 0,
+            },
+            "description": "key/value secret storage",
+            "external_entropy_access": False,
+            "local": False,
+            "options": {
+                "version": "2",
+            },
+            "path": "secret/",
+            "seal_wrap": False,
+            "type": "kv",
+            "uuid": "1d9431ac-060a-9b63-4572-3ca7ffd78347",
+        },
+        "wrap_info": None,
+        "warnings": None,
+        "auth": None,
+    }
+
+
+@pytest.fixture
+def kvv1_info():
+    return {
+        "v2": False,
+        "data": "secret/some/path",
+        "metadata": "secret/some/path",
+        "delete": "secret/some/path",
+        "type": "kv",
+    }
+
+
+@pytest.fixture
+def kvv2_info():
+    return {
+        "v2": True,
+        "data": "secret/data/some/path",
+        "metadata": "secret/metadata/some/path",
+        "delete": "secret/data/some/path",
+        "delete_versions": "secret/delete/some/path",
+        "destroy": "secret/destroy/some/path",
+        "type": "kv",
+    }
+
+
+@pytest.fixture
+def no_kv_info():
+    return {
+        "v2": False,
+        "data": "secret/some/path",
+        "metadata": "secret/some/path",
+        "delete": "secret/some/path",
+        "type": None,
+    }
+
+
+@pytest.fixture
+def kvv1_response():
+    return {
+        "request_id": "35df4df1-c3d8-b270-0682-ddb0160c7450",
+        "lease_id": "",
+        "renewable": False,
+        "lease_duration": 0,
+        "data": {
+            "foo": "bar",
+        },
+        "wrap_info": None,
+        "warnings": None,
+        "auth": None,
+    }
+
+
+@pytest.fixture
+def kvv2_response():
+    return {
+        "request_id": "35df4df1-c3d8-b270-0682-ddb0160c7450",
+        "lease_id": "",
+        "renewable": False,
+        "lease_duration": 0,
+        "data": {
+            "data": {"foo": "bar"},
+            "metadata": {
+                "created_time": "2020-05-02T07:26:12.180848003Z",
+                "deletion_time": "",
+                "destroyed": False,
+                "version": 1,
+            },
+        },
+        "wrap_info": None,
+        "warnings": None,
+        "auth": None,
+    }
+
+
+@pytest.fixture
+def kv_list_response():
+    return {
+        "request_id": "35df4df1-c3d8-b270-0682-ddb0160c7450",
+        "lease_id": "",
+        "renewable": False,
+        "lease_duration": 0,
+        "data": {
+            "keys": ["foo"],
+        },
+        "wrap_info": None,
+        "warnings": None,
+        "auth": None,
+    }
+
+
+@pytest.fixture
+def metadata_nocache():
+    cache = Mock(spec=vcache.VaultCache)
+    cache.get.return_value = None
+    return cache
+
+
+@pytest.fixture(params=["v1", "v2"])
+def kv_meta(request, metadata_nocache):
+    client = Mock(spec=vclient.AuthenticatedVaultClient)
+    if request.param == "invalid":
+        res = {"wrap_info": {}}
+    else:
+        res = request.getfixturevalue(f"kv{request.param}_meta_response")
+    client.get.return_value = res
+    return vkv.VaultKV(client, metadata_nocache)
+
+
+@pytest.fixture(params=["v1", "v2"])
+def kv_meta_cached(request):
+    cache = Mock(spec=vcache.VaultCache)
+    client = Mock(spec=vclient.AuthenticatedVaultClient)
+    kv_meta_response = request.getfixturevalue(f"kv{request.param}_meta_response")
+    client.get.return_value = kv_meta_response
+    cache.get.return_value = {"secret/some/path": kv_meta_response["data"]}
+    return vkv.VaultKV(client, cache)
+
+
+@pytest.fixture
+def kvv1(kvv1_info, kvv1_response, metadata_nocache, kv_list_response):
+    client = Mock(spec=vclient.AuthenticatedVaultClient)
+    client.get.return_value = kvv1_response
+    client.post.return_value = True
+    client.patch.side_effect = vclient.VaultPermissionDeniedError
+    client.list.return_value = kv_list_response
+    client.delete.return_value = True
+    with patch("salt.utils.vault.kv.VaultKV.is_v2", Mock(return_value=kvv1_info)):
+        yield vkv.VaultKV(client, metadata_nocache)
+
+
+@pytest.fixture
+def kvv2(kvv2_info, kvv2_response, metadata_nocache, kv_list_response):
+    client = Mock(spec=vclient.AuthenticatedVaultClient)
+    client.get.return_value = kvv2_response
+    client.post.return_value = True
+    client.patch.return_value = True
+    client.list.return_value = kv_list_response
+    client.delete.return_value = True
+    with patch("salt.utils.vault.kv.VaultKV.is_v2", Mock(return_value=kvv2_info)):
+        yield vkv.VaultKV(client, metadata_nocache)
+
+
+@pytest.mark.parametrize(
+    "wrapper,param,result",
+    [
+        ("read_kv", None, {"foo": "bar"}),
+        ("write_kv", {"foo": "bar"}, True),
+        ("patch_kv", {"foo": "bar"}, True),
+        ("delete_kv", None, True),
+        ("destroy_kv", [0], True),
+        ("list_kv", None, ["foo"]),
+    ],
+)
+@pytest.mark.parametrize("test_remote_config", ["token"], indirect=True)
+@pytest.mark.parametrize(
+    "clear_unauthd,token_valid", [(False, False), (True, False), (True, True)]
+)
+def test_kv_wrapper_handles_perm_exceptions(
+    wrapper, param, result, test_remote_config, clear_unauthd, token_valid
+):
+    """
+    Test that *_kv wrappers retry with a new client if
+      a) the current configuration might be invalid
+      b) the current token might not have all policies and
+         `cache:clear_on_unauthorized` is True
+    """
+    func = getattr(vault, wrapper)
+    exc = vault.VaultPermissionDeniedError
+    args = ["secret/some/path"]
+    if param:
+        args.append(param)
+    args += [{}, {}]
+    test_remote_config["cache"]["clear_on_unauthorized"] = clear_unauthd
+    with patch("salt.utils.vault.get_kv", autospec=True) as getkv:
+        with patch("salt.utils.vault.clear_cache", autospec=True) as cache:
+            kv = Mock(spec=vkv.VaultKV)
+            kv.client = Mock(spec=vclient.AuthenticatedVaultClient)
+            kv.client.token_valid.return_value = token_valid
+            getattr(kv, wrapper.rstrip("_kv")).side_effect = (exc, result)
+            getkv.side_effect = ((kv, test_remote_config), kv)
+            res = func(*args)
+            assert res == result
+            cache.assert_called_once()
+
+
+@pytest.mark.parametrize(
+    "wrapper,param",
+    [
+        ("read_kv", None),
+        ("write_kv", {"foo": "bar"}),
+        ("patch_kv", {"foo": "bar"}),
+        ("delete_kv", None),
+        ("destroy_kv", [0]),
+        ("list_kv", None),
+    ],
+)
+@pytest.mark.parametrize("test_remote_config", ["token"], indirect=True)
+def test_kv_wrapper_raises_perm_exceptions_when_configured(
+    wrapper, param, test_remote_config
+):
+    """
+    Test that *_kv wrappers do not retry with a new client when `cache:clear_on_unauthorized` is False.
+    """
+    func = getattr(vault, wrapper)
+    exc = vault.VaultPermissionDeniedError
+    args = ["secret/some/path"]
+    if param:
+        args.append(param)
+    args += [{}, {}]
+    test_remote_config["cache"]["clear_on_unauthorized"] = False
+    with patch("salt.utils.vault.get_kv", autospec=True) as getkv:
+        with patch("salt.utils.vault.clear_cache", autospec=True):
+            kv = Mock(spec=vkv.VaultKV)
+            kv.client = Mock(spec=vclient.AuthenticatedVaultClient)
+            kv.client.token_valid.return_value = True
+            getattr(kv, wrapper.rstrip("_kv")).side_effect = exc
+            getkv.return_value = (kv, test_remote_config)
+            with pytest.raises(exc):
+                func(*args)
+
+
+@pytest.mark.parametrize(
+    "kv_meta,expected",
+    [
+        (
+            "v1",
+            "kvv1_info",
+        ),
+        (
+            "v2",
+            "kvv2_info",
+        ),
+        (
+            "invalid",
+            "no_kv_info",
+        ),
+    ],
+    indirect=["kv_meta"],
+)
+def test_vault_kv_is_v2_no_cache(kv_meta, expected, request):
+    """
+    Ensure path metadata is requested as expected and cached
+    if the lookup succeeds
+    """
+    expected_val = request.getfixturevalue(expected)
+    res = kv_meta.is_v2("secret/some/path")
+    kv_meta.metadata_cache.get.assert_called_once()
+    kv_meta.client.get.assert_called_once_with(
+        "sys/internal/ui/mounts/secret/some/path"
+    )
+    if expected != "no_kv_info":
+        kv_meta.metadata_cache.store.assert_called_once()
+    assert res == expected_val
+
+
+@pytest.mark.parametrize(
+    "kv_meta_cached,expected",
+    [
+        (
+            "v1",
+            "kvv1_info",
+        ),
+        (
+            "v2",
+            "kvv2_info",
+        ),
+    ],
+    indirect=["kv_meta_cached"],
+)
+def test_vault_kv_is_v2_cached(kv_meta_cached, expected, request):
+    """
+    Ensure cache is respected for path metadata
+    """
+    expected = request.getfixturevalue(expected)
+    res = kv_meta_cached.is_v2("secret/some/path")
+    kv_meta_cached.metadata_cache.get.assert_called_once()
+    kv_meta_cached.metadata_cache.store.assert_not_called()
+    kv_meta_cached.client.assert_not_called()
+    assert res == expected
+
+
+class TestKVV1:
+    path = "secret/some/path"
+
+    @pytest.mark.parametrize("include_metadata", [False, True])
+    def test_vault_kv_read(self, kvv1, include_metadata, path):
+        """
+        Ensure that VaultKV.read works for KV v1 and does not fail if
+        metadata is requested, which is invalid for KV v1.
+        """
+        res = kvv1.read(path, include_metadata=include_metadata)
+        kvv1.client.get.assert_called_once_with(path)
+        assert res == {"foo": "bar"}
+
+    def test_vault_kv_write(self, kvv1, path):
+        """
+        Ensure that VaultKV.write works for KV v1.
+        """
+        data = {"bar": "baz"}
+        kvv1.write(path, data)
+        kvv1.client.post.assert_called_once_with(path, payload=data)
+
+    @pytest.mark.parametrize(
+        "existing,data,expected",
+        [
+            ({"foo": "bar"}, {"bar": "baz"}, {"foo": "bar", "bar": "baz"}),
+            ({"foo": "bar"}, {"foo": None}, {}),
+            (
+                {"foo": "bar"},
+                {"foo2": {"bar": {"baz": True}}},
+                {"foo": "bar", "foo2": {"bar": {"baz": True}}},
+            ),
+            (
+                {"foo": {"bar": {"baz": True}}},
+                {"foo": {"bar": {"baz": None}}},
+                {"foo": {"bar": {}}},
+            ),
+        ],
+    )
+    def test_vault_kv_patch(self, kvv1, path, existing, data, expected):
+        """
+        Ensure that VaultKV.patch works for KV v1.
+        This also tests the internal JSON merge patch implementation.
+        """
+        kvv1.client.get.return_value = {"data": existing}
+        kvv1.patch(path, data)
+        kvv1.client.post.assert_called_once_with(
+            path,
+            payload=expected,
+        )
+
+    def test_vault_kv_delete(self, kvv1, path):
+        """
+        Ensure that VaultKV.delete works for KV v1.
+        """
+        kvv1.delete(path)
+        kvv1.client.request.assert_called_once_with("DELETE", path, payload=None)
+
+    def test_vault_kv_delete_versions(self, kvv1, path):
+        """
+        Ensure that VaultKV.delete with versions raises an exception for KV v1.
+        """
+        with pytest.raises(
+            vault.VaultInvocationError, match="Versioning support requires kv-v2.*"
+        ):
+            kvv1.delete(path, versions=[1, 2, 3, 4])
+
+    def test_vault_kv_destroy(self, kvv1, path):
+        """
+        Ensure that VaultKV.destroy raises an exception for KV v1.
+        """
+        with pytest.raises(vault.VaultInvocationError):
+            kvv1.destroy(path, [1, 2, 3, 4])
+
+    def test_vault_kv_nuke(self, kvv1, path):
+        """
+        Ensure that VaultKV.nuke raises an exception for KV v1.
+        """
+        with pytest.raises(vault.VaultInvocationError):
+            kvv1.nuke(path)
+
+    def test_vault_kv_list(self, kvv1, path):
+        """
+        Ensure that VaultKV.list works for KV v1 and only returns keys.
+        """
+        res = kvv1.list(path)
+        kvv1.client.list.assert_called_once_with(path)
+        assert res == ["foo"]
+
+
+class TestKVV2:
+    @pytest.mark.parametrize(
+        "versions,expected",
+        [
+            (0, [0]),
+            ("1", [1]),
+            ([2], [2]),
+            (["3"], [3]),
+        ],
+    )
+    def test_parse_versions(self, kvv2, versions, expected):
+        """
+        Ensure parsing versions works as expected:
+        single integer/number string or list of those are allowed
+        """
+        assert kvv2._parse_versions(versions) == expected
+
+    def test_parse_versions_raises_exception_when_unparsable(self, kvv2):
+        """
+        Ensure unparsable versions raise an exception
+        """
+        with pytest.raises(vault.VaultInvocationError):
+            kvv2._parse_versions("four")
+
+    def test_get_secret_path_metadata_lookup_unexpected_response(
+        self, kvv2, caplog, path
+    ):
+        """
+        Ensure unexpected responses are treated as not KV
+        """
+        # _mock_json_response() returns a Mock, but we need MagicMock here
+        resp_mm = MagicMock(spec=requests.models.Response)
+        resp_mm.json.return_value = {"wrap_info": {}}
+        resp_mm.status_code = 200
+        resp_mm.reason = ""
+        kvv2.client.get.return_value = resp_mm
+        res = kvv2._get_secret_path_metadata(path)
+        assert res is None
+        assert "Unexpected response to metadata query" in caplog.text
+
+    def test_get_secret_path_metadata_lookup_request_error(self, kvv2, caplog, path):
+        """
+        Ensure HTTP error status codes are treated as not KV
+        """
+        kvv2.client.get.side_effect = vault.VaultPermissionDeniedError
+        res = kvv2._get_secret_path_metadata(path)
+        assert res is None
+        assert "VaultPermissionDeniedError:" in caplog.text
+
+    @pytest.mark.parametrize("include_metadata", [False, True])
+    def test_vault_kv_read(self, kvv2, include_metadata, kvv2_response, paths):
+        """
+        Ensure that VaultKV.read works for KV v2 and returns metadata
+        if requested.
+        """
+        res = kvv2.read(path, include_metadata=include_metadata)
+        kvv2.client.get.assert_called_once_with(paths["data"])
+        if include_metadata:
+            assert res == kvv2_response["data"]
+        else:
+            assert res == kvv2_response["data"]["data"]
+
+    def test_vault_kv_write(self, kvv2, path, paths):
+        """
+        Ensure that VaultKV.write works for KV v2.
+        """
+        data = {"bar": "baz"}
+        kvv2.write(path, data)
+        kvv2.client.post.assert_called_once_with(paths["data"], payload={"data": data})
+
+    def test_vault_kv_patch(self, kvv2, path, paths):
+        """
+        Ensure that VaultKV.patch works for KV v2.
+        """
+        data = {"bar": "baz"}
+        kvv2.patch(path, data)
+        kvv2.client.patch.assert_called_once_with(
+            paths["data"],
+            payload={"data": data},
+            add_headers={"Content-Type": "application/merge-patch+json"},
+        )
+
+    def test_vault_kv_delete(self, kvv2, path, paths):
+        """
+        Ensure that VaultKV.delete works for KV v2.
+        """
+        kvv2.delete(path)
+        kvv2.client.request.assert_called_once_with(
+            "DELETE", paths["data"], payload=None
+        )
+
+    @pytest.mark.parametrize(
+        "versions", [[1, 2], [2], 2, ["1", "2"], ["2"], "2", [1, "2"]]
+    )
+    def test_vault_kv_delete_versions(self, kvv2, versions, path, paths):
+        """
+        Ensure that VaultKV.delete with versions works for KV v2.
+        """
+        if isinstance(versions, list):
+            expected = [int(x) for x in versions]
+        else:
+            expected = [int(versions)]
+        kvv2.delete(path, versions=versions)
+        kvv2.client.request.assert_called_once_with(
+            "POST", paths["delete_versions"], payload={"versions": expected}
+        )
+
+    @pytest.mark.parametrize(
+        "versions", [[1, 2], [2], 2, ["1", "2"], ["2"], "2", [1, "2"]]
+    )
+    def test_vault_kv_destroy(self, kvv2, versions, path, paths):
+        """
+        Ensure that VaultKV.destroy works for KV v2.
+        """
+        if isinstance(versions, list):
+            expected = [int(x) for x in versions]
+        else:
+            expected = [int(versions)]
+        kvv2.destroy(path, versions)
+        kvv2.client.post.assert_called_once_with(
+            paths["destroy"], payload={"versions": expected}
+        )
+
+    def test_vault_kv_nuke(self, kvv2, path, paths):
+        """
+        Ensure that VaultKV.nuke works for KV v2.
+        """
+        kvv2.nuke(path)
+        kvv2.client.delete.assert_called_once_with(paths["metadata"])
+
+    def test_vault_kv_list(self, kvv2, path, paths):
+        """
+        Ensure that VaultKV.list works for KV v2 and only returns keys.
+        """
+        res = kvv2.list(path)
+        kvv2.client.list.assert_called_once_with(paths["metadata"])
+        assert res == ["foo"]

tests/pytests/unit/utils/vault/test_leases.py

@@ -0,0 +1,363 @@
+import pytest
+
+import salt.utils.vault as vault
+import salt.utils.vault.cache as vcache
+import salt.utils.vault.client as vclient
+import salt.utils.vault.leases as leases
+from tests.support.mock import Mock, call, patch
+
+
+@pytest.fixture(autouse=True, params=[0])
+def time_stopped(request):
+    with patch(
+        "salt.utils.vault.leases.time.time", autospec=True, return_value=request.param
+    ):
+        yield
+
+
+@pytest.fixture
+def lease_renewed_response():
+    return {
+        "lease_id": "database/creds/testrole/abcd",
+        "renewable": True,
+        "lease_duration": 2000,
+    }
+
+
+@pytest.fixture
+def lease_renewed_extended_response():
+    return {
+        "lease_id": "database/creds/testrole/abcd",
+        "renewable": True,
+        "lease_duration": 3000,
+    }
+
+
+@pytest.fixture
+def store(events):
+    client = Mock(spec=vclient.AuthenticatedVaultClient)
+    cache = Mock(spec=vcache.VaultLeaseCache)
+    cache.exists.return_value = False
+    cache.get.return_value = None
+    return leases.LeaseStore(client, cache, expire_events=events)
+
+
+@pytest.fixture
+def store_valid(store, lease, lease_renewed_response):
+    store.cache.exists.return_value = True
+    store.cache.get.return_value = leases.VaultLease(**lease)
+    store.client.post.return_value = lease_renewed_response
+    return store
+
+
+@pytest.mark.parametrize(
+    "creation_time",
+    [
+        1661188581,
+        "1661188581",
+        "2022-08-22T17:16:21.473219641+00:00",
+        "2022-08-22T17:16:21.47321964+00:00",
+        "2022-08-22T17:16:21.4732196+00:00",
+        "2022-08-22T17:16:21.473219+00:00",
+        "2022-08-22T17:16:21.47321+00:00",
+        "2022-08-22T17:16:21.4732+00:00",
+        "2022-08-22T17:16:21.473+00:00",
+        "2022-08-22T17:16:21.47+00:00",
+        "2022-08-22T17:16:21.4+00:00",
+    ],
+)
+def test_vault_lease_creation_time_normalization(creation_time):
+    """
+    Ensure the normalization of different creation_time formats works as expected -
+    many token endpoints report a timestamp, while other endpoints report RFC3339-formatted
+    strings that may have a variable number of digits for sub-second precision (0 omitted)
+    while datetime.fromisoformat expects exactly 6 digits
+    """
+    data = {
+        "lease_id": "id",
+        "renewable": False,
+        "lease_duration": 1337,
+        "creation_time": creation_time,
+        "data": None,
+    }
+    res = leases.VaultLease(**data)
+    assert res.creation_time == 1661188581
+
+
+@pytest.mark.parametrize(
+    "time_stopped,duration,offset,expected",
+    [
+        (0, 50, 0, True),
+        (50, 10, 0, False),
+        (0, 60, 10, True),
+        (0, 60, 600, False),
+    ],
+    indirect=["time_stopped"],
+)
+def test_vault_lease_is_valid_accounts_for_time(duration, offset, expected):
+    """
+    Ensure lease validity is checked correctly and can look into the future
+    """
+    data = {
+        "lease_id": "id",
+        "renewable": False,
+        "lease_duration": duration,
+        "creation_time": 0,
+        "expire_time": duration,
+        "data": None,
+    }
+    res = leases.VaultLease(**data)
+    assert res.is_valid_for(offset) is expected
+
+
+@pytest.mark.parametrize(
+    "time_stopped,duration,offset,expected",
+    [
+        (0, 50, 0, True),
+        (50, 10, 0, False),
+        (0, 60, 10, True),
+        (0, 60, 600, False),
+    ],
+    indirect=["time_stopped"],
+)
+def test_vault_token_is_valid_accounts_for_time(duration, offset, expected):
+    """
+    Ensure token time validity is checked correctly and can look into the future
+    """
+    data = {
+        "client_token": "id",
+        "renewable": False,
+        "lease_duration": duration,
+        "num_uses": 0,
+        "creation_time": 0,
+        "expire_time": duration,
+    }
+    res = vault.VaultToken(**data)
+    assert res.is_valid_for(offset) is expected
+
+
+@pytest.mark.parametrize(
+    "num_uses,uses,expected",
+    [(0, 999999, True), (1, 0, True), (1, 1, False), (1, 2, False)],
+)
+def test_vault_token_is_valid_accounts_for_num_uses(num_uses, uses, expected):
+    """
+    Ensure token uses validity is checked correctly
+    """
+    data = {
+        "client_token": "id",
+        "renewable": False,
+        "lease_duration": 0,
+        "num_uses": num_uses,
+        "creation_time": 0,
+        "use_count": uses,
+    }
+    with patch(
+        "salt.utils.vault.leases.BaseLease.is_valid_for",
+        autospec=True,
+        return_value=True,
+    ):
+        res = vault.VaultToken(**data)
+        assert res.is_valid() is expected
+
+
+@pytest.mark.parametrize(
+    "time_stopped,duration,offset,expected",
+    [
+        (0, 50, 0, True),
+        (50, 10, 0, False),
+        (0, 60, 10, True),
+        (0, 60, 600, False),
+    ],
+    indirect=["time_stopped"],
+)
+def test_vault_approle_secret_id_is_valid_accounts_for_time(duration, offset, expected):
+    """
+    Ensure secret ID time validity is checked correctly and can look into the future
+    """
+    data = {
+        "secret_id": "test-secret-id",
+        "renewable": False,
+        "creation_time": 0,
+        "expire_time": duration,
+        "secret_id_num_uses": 0,
+        "secret_id_ttl": duration,
+    }
+    res = vault.VaultSecretId(**data)
+    assert res.is_valid(offset) is expected
+
+
+@pytest.mark.parametrize(
+    "num_uses,uses,expected",
+    [(0, 999999, True), (1, 0, True), (1, 1, False), (1, 2, False)],
+)
+def test_vault_approle_secret_id_is_valid_accounts_for_num_uses(
+    num_uses, uses, expected
+):
+    """
+    Ensure secret ID uses validity is checked correctly
+    """
+    data = {
+        "secret_id": "test-secret-id",
+        "renewable": False,
+        "creation_time": 0,
+        "secret_id_ttl": 0,
+        "secret_id_num_uses": num_uses,
+        "use_count": uses,
+    }
+    with patch(
+        "salt.utils.vault.leases.BaseLease.is_valid_for",
+        autospec=True,
+        return_value=True,
+    ):
+        res = vault.VaultSecretId(**data)
+        assert res.is_valid() is expected
+
+
+class TestLeaseStore:
+    def test_get_uncached_or_invalid(self, store):
+        """
+        Ensure uncached or invalid leases are reported as None.
+        """
+        ret = store.get("test")
+        assert ret is None
+        store.client.post.assert_not_called()
+        store.cache.flush.assert_not_called()
+        store.cache.store.assert_not_called()
+
+    def test_get_cached_valid(self, store_valid, lease):
+        """
+        Ensure valid leases are returned without extra behavior.
+        """
+        ret = store_valid.get("test")
+        assert ret == lease
+        store_valid.client.post.assert_not_called()
+        store_valid.cache.flush.assert_not_called()
+        store_valid.cache.store.assert_not_called()
+
+    @pytest.mark.parametrize(
+        "valid_for", [2000, pytest.param(2002, id="2002_renewal_leeway")]
+    )
+    def test_get_valid_renew_default_period(self, store_valid, lease, valid_for):
+        """
+        Ensure renewals are attempted by default, cache is updated accordingly
+        and validity checks after renewal allow for a little leeway to account
+        for latency.
+        """
+        ret = store_valid.get("test", valid_for=valid_for)
+        lease["duration"] = lease["expire_time"] = 2000
+        assert ret == lease
+        store_valid.client.post.assert_called_once_with(
+            "sys/leases/renew", payload={"lease_id": lease["id"]}
+        )
+        store_valid.cache.flush.assert_not_called()
+        store_valid.cache.store.assert_called_once_with("test", ret)
+        store_valid.expire_events.assert_not_called()
+
+    def test_get_valid_renew_increment(self, store_valid, lease):
+        """
+        Ensure renew_increment is honored when renewing.
+        """
+        ret = store_valid.get("test", valid_for=1400, renew_increment=2000)
+        lease["duration"] = lease["expire_time"] = 2000
+        assert ret == lease
+        store_valid.client.post.assert_called_once_with(
+            "sys/leases/renew", payload={"lease_id": lease["id"], "increment": 2000}
+        )
+        store_valid.cache.flush.assert_not_called()
+        store_valid.cache.store.assert_called_once_with("test", ret)
+        store_valid.expire_events.assert_not_called()
+
+    def test_get_valid_renew_increment_insufficient(self, store_valid, lease):
+        """
+        Ensure that when renewal_increment is set, valid_for is respected and that
+        a second renewal using valid_for as increment is not attempted when the
+        Vault server does not allow renewals for at least valid_for.
+        If an event factory was passed, an event should be sent.
+        """
+        ret = store_valid.get("test", valid_for=2100, renew_increment=3000)
+        assert ret is None
+        store_valid.client.post.assert_has_calls(
+            (
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"], "increment": 3000},
+                ),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"], "increment": 60},
+                ),
+            )
+        )
+        store_valid.cache.flush.assert_called_once_with("test")
+        store_valid.expire_events.assert_called_once_with(
+            tag="vault/lease/test/expire", data={"valid_for_less": 2100}
+        )
+
+    @pytest.mark.parametrize(
+        "valid_for", [3000, pytest.param(3002, id="3002_renewal_leeway")]
+    )
+    def test_get_valid_renew_valid_for(
+        self,
+        store_valid,
+        lease,
+        valid_for,
+        lease_renewed_response,
+        lease_renewed_extended_response,
+    ):
+        """
+        Ensure that, if renew_increment was not set and the default period
+        does not yield valid_for, a second renewal is attempted by valid_for.
+        There should be some leeway by default to account for latency.
+        """
+        store_valid.client.post.side_effect = (
+            lease_renewed_response,
+            lease_renewed_extended_response,
+        )
+        ret = store_valid.get("test", valid_for=valid_for)
+        lease["duration"] = lease["expire_time"] = 3000
+        assert ret == lease
+        store_valid.client.post.assert_has_calls(
+            (
+                call("sys/leases/renew", payload={"lease_id": lease["id"]}),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"], "increment": valid_for},
+                ),
+            )
+        )
+        store_valid.cache.flush.assert_not_called()
+        store_valid.cache.store.assert_called_with("test", ret)
+        store_valid.expire_events.assert_not_called()
+
+    def test_get_valid_not_renew(self, store_valid, lease):
+        """
+        Currently valid leases should not be returned if they undercut
+        valid_for. By default, revocation should be attempted and cache
+        should be flushed. If an event factory was passed, an event should be sent.
+        """
+        ret = store_valid.get("test", valid_for=2000, renew=False)
+        assert ret is None
+        store_valid.cache.store.assert_not_called()
+        store_valid.client.post.assert_called_once_with(
+            "sys/leases/renew", payload={"lease_id": lease["id"], "increment": 60}
+        )
+        store_valid.cache.flush.assert_called_once_with("test")
+        store_valid.expire_events.assert_called_once_with(
+            tag="vault/lease/test/expire", data={"valid_for_less": 2000}
+        )
+
+    def test_get_valid_not_flush(self, store_valid):
+        """
+        Currently valid leases should not be returned if they undercut
+        valid_for and should not be revoked if requested so.
+        If an event factory was passed, an event should be sent.
+        """
+        ret = store_valid.get("test", valid_for=2000, revoke=False, renew=False)
+        assert ret is None
+        store_valid.cache.flush.assert_not_called()
+        store_valid.client.post.assert_not_called()
+        store_valid.cache.store.assert_not_called()
+        store_valid.expire_events.assert_called_once_with(
+            tag="vault/lease/test/expire", data={"valid_for_less": 2000}
+        )

tests/support/pytest/vault.py

@@ -0,0 +1,311 @@
+import json
+import logging
+import subprocess
+import time
+
+import pytest
+from pytestshellutils.utils.processes import ProcessResult
+
+import salt.utils.files
+import salt.utils.path
+from tests.support.helpers import PatchedEnviron
+from tests.support.runtests import RUNTIME_VARS
+
+log = logging.getLogger(__name__)
+
+
+def _vault_cmd(cmd, textinput=None, raw=False):
+    vault_binary = salt.utils.path.which("vault")
+    proc = subprocess.run(
+        [vault_binary] + cmd,
+        check=False,
+        input=textinput,
+        capture_output=True,
+        text=True,
+    )
+
+    ret = ProcessResult(
+        returncode=proc.returncode,
+        stdout=proc.stdout,
+        stderr=proc.stderr,
+        cmdline=proc.args,
+    )
+
+    if raw:
+        return ret
+    if ret.returncode != 0:
+        log.debug("Failed to run vault %s:\n%s", " ".join(cmd), ret)
+        raise RuntimeError()
+    return ret
+
+
+def vault_write_policy(name, rules):
+    try:
+        _vault_cmd(["policy", "write", name, "-"], textinput=rules)
+    except RuntimeError:
+        pytest.fail(f"Unable to write policy `{name}`")
+
+
+def vault_write_policy_file(policy, filename=None):
+    if filename is None:
+        filename = policy
+    try:
+        _vault_cmd(
+            [
+                "policy",
+                "write",
+                policy,
+                f"{RUNTIME_VARS.FILES}/vault/policies/{filename}.hcl",
+            ]
+        )
+    except RuntimeError:
+        pytest.fail(f"Unable to write policy `{policy}`")
+
+
+def vault_read_policy(policy):
+    ret = _vault_cmd(["policy", "read", "-format=json", policy], raw=True)
+    if ret.returncode != 0:
+        if "No policy named" in ret.stderr:
+            return None
+        log.debug("Failed to read policy `%s`:\n%s", policy, ret)
+        pytest.fail(f"Unable to read policy `{policy}`")
+    res = json.loads(ret.stdout)
+    return res["policy"]
+
+
+def vault_list_policies():
+    try:
+        ret = _vault_cmd(["policy", "list", "-format=json"])
+    except RuntimeError:
+        pytest.fail("Unable to list policies")
+    return json.loads(ret.stdout)
+
+
+def vault_delete_policy(policy):
+    try:
+        _vault_cmd(["policy", "delete", policy])
+    except RuntimeError:
+        pytest.fail(f"Unable to delete policy `{policy}`")
+
+
+def vault_enable_secret_engine(name, options=None, **kwargs):
+    if options is None:
+        options = []
+    try:
+        ret = _vault_cmd(["secrets", "enable"] + options + [name])
+    except RuntimeError:
+        pytest.fail(f"Could not enable secret engine `{name}`")
+
+    if "path is already in use at" in ret.stdout:
+        return False
+    if "Success" in ret.stdout:
+        return True
+    log.debug("Failed to enable secret engine `%s`:\n%s", name, ret)
+    pytest.fail(f"Could not enable secret engine `{name}`: {ret.stdout}")
+
+
+def vault_disable_secret_engine(name):
+    try:
+        ret = _vault_cmd(["secrets", "disable", name])
+    except RuntimeError:
+        pytest.fail(f"Could not disable secret engine `{name}`")
+
+    if "Success" in ret.stdout:
+        return True
+    log.debug("Failed to disable secret engine `%s`:\n%s", name, ret)
+    pytest.fail(f"Could not disable secret engine `{name}`: {ret.stdout}")
+
+
+def vault_enable_auth_method(name, options=None, **kwargs):
+    if options is None:
+        options = []
+    cmd = (
+        ["auth", "enable"] + options + [name] + [f"{k}={v}" for k, v in kwargs.items()]
+    )
+    try:
+        ret = _vault_cmd(cmd)
+    except RuntimeError:
+        pytest.fail(f"Could not enable auth method `{name}`")
+
+    if "path is already in use at" in ret.stdout:
+        return False
+    if "Success" in ret.stdout:
+        return True
+    log.debug("Failed to enable auth method `%s`:\n%s", name, ret)
+    pytest.fail(f"Could not enable auth method `{name}`: {ret.stdout}")
+
+
+def vault_disable_auth_method(name):
+    try:
+        ret = _vault_cmd(["auth", "disable", name])
+    except RuntimeError:
+        pytest.fail(f"Could not disable auth method `{name}`")
+
+    if "Success" in ret.stdout:
+        return True
+    log.debug("Failed to disable auth method `%s`:\n%s", name, ret)
+    pytest.fail(f"Could not disable auth method `{name}`: {ret.stdout}")
+
+
+def vault_write_secret(path, **kwargs):
+    cmd = ["kv", "put", path] + [f"{k}={v}" for k, v in kwargs.items()]
+    try:
+        ret = _vault_cmd(cmd)
+    except RuntimeError:
+        pytest.fail(f"Failed to write secret at `{path}`")
+
+    if vault_read_secret(path) != kwargs:
+        log.debug("Failed to write secret at `%s`:\n%s", path, ret)
+        pytest.fail(f"Failed to write secret at `{path}`")
+    return True
+
+
+def vault_write_secret_file(path, data_name):
+    data_path = f"{RUNTIME_VARS.FILES}/vault/data/{data_name}.json"
+    with salt.utils.files.fopen(data_path) as f:
+        data = json.load(f)
+    cmd = ["kv", "put", path, f"@{data_path}"]
+    try:
+        ret = _vault_cmd([cmd])
+    except RuntimeError:
+        pytest.fail(f"Failed to write secret at `{path}`")
+
+    if vault_read_secret(path) != data:
+        log.debug("Failed to write secret at `%s`:\n%s", path, ret)
+        pytest.fail(f"Failed to write secret at `{path}`")
+    return True
+
+
+def vault_read_secret(path):
+    ret = _vault_cmd(["kv", "get", "-format=json", path], raw=True)
+
+    if ret.returncode != 0:
+        if "No value found at" in ret.stderr:
+            return None
+        log.debug("Failed to read secret at `%s`:\n%s", path, ret)
+        pytest.fail(f"Failed to read secret at `{path}`")
+    res = json.loads(ret.stdout)
+    if "data" in res["data"]:
+        return res["data"]["data"]
+    return res["data"]
+
+
+def vault_list_secrets(path):
+    ret = _vault_cmd(["kv", "list", "-format=json", path], raw=True)
+    if ret.returncode != 0:
+        if ret.returncode == 2:
+            return []
+        log.debug("Failed to list secrets at `%s`:\n%s", path, ret)
+        pytest.fail(f"Failed to list secrets at `{path}`")
+    return json.loads(ret.stdout)
+
+
+def vault_delete_secret(path, metadata=False):
+    try:
+        ret = _vault_cmd(["kv", "delete", path])
+    except RuntimeError:
+        pytest.fail(f"Failed to delete secret at `{path}`")
+
+    if vault_read_secret(path) is not None:
+        log.debug("Failed to delete secret at `%s`:\n%s", path, ret)
+        pytest.fail(f"Failed to delete secret at `{path}`")
+
+    if not metadata:
+        return True
+
+    ret = _vault_cmd(["kv", "metadata", "delete", path], raw=True)
+    if (
+        ret.returncode != 0
+        and "Metadata not supported on KV Version 1" not in ret.stderr
+    ):
+        log.debug("Failed to delete secret metadata at `%s`:\n%s", path, ret)
+        pytest.fail(f"Failed to delete secret metadata at `{path}`")
+    return True
+
+
+def vault_list(path):
+    try:
+        ret = _vault_cmd(["list", "-format=json", path])
+    except RuntimeError:
+        pytest.fail(f"Failed to list path at `{path}`")
+    return json.loads(ret.stdout)
+
+
+@pytest.fixture(scope="module")
+def vault_environ(vault_port):
+    with PatchedEnviron(VAULT_ADDR=f"http://127.0.0.1:{vault_port}"):
+        yield
+
+
+def vault_container_version_id(value):
+    return f"vault=={value}"
+
+
+@pytest.fixture(
+    scope="module",
+    params=["0.9.6", "1.3.1", "latest"],
+    ids=vault_container_version_id,
+)
+def vault_container_version(request, salt_factories, vault_port, vault_environ):
+    vault_version = request.param
+    vault_binary = salt.utils.path.which("vault")
+    config = {
+        "backend": {"file": {"path": "/vault/file"}},
+        "default_lease_ttl": "168h",
+        "max_lease_ttl": "720h",
+    }
+
+    factory = salt_factories.get_container(
+        "vault",
+        f"ghcr.io/saltstack/salt-ci-containers/vault:{vault_version}",
+        check_ports=[vault_port],
+        container_run_kwargs={
+            "ports": {"8200/tcp": vault_port},
+            "environment": {
+                "VAULT_DEV_ROOT_TOKEN_ID": "testsecret",
+                "VAULT_LOCAL_CONFIG": json.dumps(config),
+            },
+            "cap_add": "IPC_LOCK",
+        },
+        pull_before_start=True,
+        skip_on_pull_failure=True,
+        skip_if_docker_client_not_connectable=True,
+    )
+    with factory.started() as factory:
+        attempts = 0
+        while attempts < 3:
+            attempts += 1
+            time.sleep(1)
+            proc = subprocess.run(
+                [vault_binary, "login", "token=testsecret"],
+                check=False,
+                capture_output=True,
+                text=True,
+            )
+            if proc.returncode == 0:
+                break
+            ret = ProcessResult(
+                returncode=proc.returncode,
+                stdout=proc.stdout,
+                stderr=proc.stderr,
+                cmdline=proc.args,
+            )
+            log.debug("Failed to authenticate against vault:\n%s", ret)
+            time.sleep(4)
+        else:
+            pytest.fail("Failed to login to vault")
+
+        vault_write_policy_file("salt_master")
+
+        if "latest" == vault_version:
+            vault_write_policy_file("salt_minion")
+        else:
+            vault_write_policy_file("salt_minion", "salt_minion_old")
+
+        if vault_version in ("1.3.1", "latest"):
+            vault_enable_secret_engine("kv-v2")
+            if vault_version == "latest":
+                vault_enable_auth_method("approle", ["-path=salt-minions"])
+                vault_enable_secret_engine("kv", ["-version=2", "-path=salt"])
+
+        yield vault_version