From 984a8d2db4947e12966abc941962b9ae8c3927cb Mon Sep 17 00:00:00 2001 From: jeanluc Date: Mon, 11 Dec 2023 12:55:55 +0100 Subject: [PATCH] Rework deprecations We're independent from Salt releases now, so act like it. --- docs/all.rst | 6 ++ docs/ref/utils/index.rst | 12 +++ .../utils/saltext.vault.utils.versions.rst | 5 + src/saltext/vault/modules/vault.py | 20 +++- src/saltext/vault/runners/vault.py | 11 ++- src/saltext/vault/sdb/vault.py | 22 ++++- src/saltext/vault/utils/versions.py | 95 +++++++++++++++++++ .../vault/test_token_auth_deprecated.py | 2 +- 8 files changed, 165 insertions(+), 8 deletions(-) create mode 100644 docs/ref/utils/index.rst create mode 100644 docs/ref/utils/saltext.vault.utils.versions.rst create mode 100644 src/saltext/vault/utils/versions.py diff --git a/docs/all.rst b/docs/all.rst index f039116f..57e2cc78 100644 --- a/docs/all.rst +++ b/docs/all.rst @@ -32,3 +32,9 @@ Complete List of saltext-vault :maxdepth: 2 ref/states/index + + +.. toctree:: + :maxdepth: 2 + + ref/utils/index diff --git a/docs/ref/utils/index.rst b/docs/ref/utils/index.rst new file mode 100644 index 00000000..4458976e --- /dev/null +++ b/docs/ref/utils/index.rst @@ -0,0 +1,12 @@ +.. all-saltext.vault.utils: + +____________ +Util Modules +____________ + +.. currentmodule:: saltext.vault.utils + +.. autosummary:: + :toctree: + + versions diff --git a/docs/ref/utils/saltext.vault.utils.versions.rst b/docs/ref/utils/saltext.vault.utils.versions.rst new file mode 100644 index 00000000..c14562cc --- /dev/null +++ b/docs/ref/utils/saltext.vault.utils.versions.rst @@ -0,0 +1,5 @@ +saltext.vault.utils.versions +============================ + +.. automodule:: saltext.vault.utils.versions + :members: diff --git a/src/saltext/vault/modules/vault.py b/src/saltext/vault/modules/vault.py index 59d79bb2..58bfbab5 100644 --- a/src/saltext/vault/modules/vault.py +++ b/src/saltext/vault/modules/vault.py @@ -698,6 +698,7 @@ from salt.exceptions import CommandExecutionError from salt.exceptions import SaltException from salt.exceptions import SaltInvocationError +from saltext.vault.utils.versions import warn_until log = logging.getLogger(__name__) @@ -972,7 +973,7 @@ def destroy_secret(path, *args): return False -def list_secrets(path, default=NOT_SET, keys_only=False): +def list_secrets(path, default=NOT_SET, keys_only=None): """ List secret keys at . The vault policy used must allow this. The path should end with a trailing slash. @@ -1008,10 +1009,25 @@ def list_secrets(path, default=NOT_SET, keys_only=False): This function used to return a dictionary like ``{"keys": ["some/", "some/key"]}``. Setting this to True will only return the list of keys. - For backwards-compatibility reasons, this defaults to False. + For backwards-compatibility reasons, this currently defaults to False. + Beginning with version 2 of this extension, the default will change to True. """ if default == NOT_SET: default = CommandExecutionError + if keys_only is None: + try: + warn_until( + 2, + ( + "In version {version}, this function will return the list of " + "secret keys only. You can switch to the new behavior explicitly " + "by specifying keys_only=True." + ), + ) + keys_only = False + except RuntimeError: + keys_only = True + log.debug("Listing vault secret keys for %s in %s", __grains__.get("id"), path) try: keys = vault.list_kv(path, __opts__, __context__) diff --git a/src/saltext/vault/runners/vault.py b/src/saltext/vault/runners/vault.py index 3749b4b8..528455a2 100644 --- a/src/saltext/vault/runners/vault.py +++ b/src/saltext/vault/runners/vault.py @@ -27,6 +27,7 @@ from salt.defaults import NOT_SET from salt.exceptions import SaltInvocationError from salt.exceptions import SaltRunnerError +from saltext.vault.utils.versions import warn_until log = logging.getLogger(__name__) @@ -156,9 +157,13 @@ def generate_token( ) _validate_signature(minion_id, signature, impersonated_by_master) try: - salt.utils.versions.warn_until( - "Argon", - "vault.generate_token endpoint is deprecated. Please update your minions.", + warn_until( + 2, + ( + "The vault.generate_token endpoint is deprecated and will be removed " + "in version {version}. Please ensure your minions are running the " + "Vault Salt extension as well." + ), ) if _config("issue:type") != "token": diff --git a/src/saltext/vault/sdb/vault.py b/src/saltext/vault/sdb/vault.py index 8e1e0a43..977bcc87 100644 --- a/src/saltext/vault/sdb/vault.py +++ b/src/saltext/vault/sdb/vault.py @@ -45,7 +45,8 @@ 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. + Currently defaults to ``False`` for backwards-compatibility reasons. + Beginning with version 2 of this extension, will default to ``True``. .. versionadded:: 1.0.0 """ @@ -53,6 +54,7 @@ import salt.exceptions import saltext.vault.utils.vault as vault +from saltext.vault.utils.versions import warn_until log = logging.getLogger(__name__) @@ -70,8 +72,24 @@ def set_(key, value, profile=None): # pylint: disable=unused-argument data = {key: value} curr_data = {} profile = profile or {} + patch = profile.get("patch") - if profile.get("patch"): + if patch is None: + try: + warn_until( + 2, + ( + "Beginning with version {version}, the Vault SDB module will " + "partially update secrets instead of overwriting it completely. " + "You can switch to the new behavior explicitly by specifying " + "patch: true in your Vault SDB configuration." + ), + ) + patch = False + except RuntimeError: + patch = True + + if patch: try: # Patching only works on existing secrets. # Save the current data if patching is enabled diff --git a/src/saltext/vault/utils/versions.py b/src/saltext/vault/utils/versions.py new file mode 100644 index 00000000..e3e4ef85 --- /dev/null +++ b/src/saltext/vault/utils/versions.py @@ -0,0 +1,95 @@ +import inspect +import os +import sys +import warnings + +import packaging.version +from saltext.vault import __version__ + + +def warn_until( + version, + message, + category=DeprecationWarning, +): + """ + Warn about deprecations until the specified version is reached, after which + raise a RuntimeError to remind developers about removal. + + Loosely based on ``salt.utils.versions.warn_until``. + + version + The version at which the warning turns into an error. Can be specified + as a string, float, int or iterable with items castable to integers. + + message + The warning message to show. + + category + The warning class to be thrown, by default ``DeprecationWarning``. + """ + version = _parse_version(version) + saltext_version = _parse_version(__version__) + # Attribute the warning to the calling function, not to warn_until() + stacklevel = 2 + + if saltext_version >= version: + caller = inspect.getframeinfo(sys._getframe(stacklevel - 1)) + raise RuntimeError( + f"The warning triggered on filename '{caller.filename}', line number " + f"{caller.lineno}, is supposed to be shown until version " + f"{version} is released. Current version is now " + f"{saltext_version}. Please remove the warning." + ) + + if os.environ.get("PYTHONWARNINGS") != "ignore": + warnings.warn( + message.format(version=version), + category, + stacklevel=stacklevel, + ) + + +class Version(packaging.version.Version): + def __lt__(self, other): + if isinstance(other, str): + other = Version(other) + return super().__lt__(other) + + def __le__(self, other): + if isinstance(other, str): + other = Version(other) + return super().__le__(other) + + def __eq__(self, other): + if isinstance(other, str): + other = Version(other) + return super().__eq__(other) + + def __ge__(self, other): + if isinstance(other, str): + other = Version(other) + return super().__ge__(other) + + def __gt__(self, other): + if isinstance(other, str): + other = Version(other) + return super().__gt__(other) + + def __ne__(self, other): + if isinstance(other, str): + other = Version(other) + return super().__ne__(other) + + +def _parse_version(version): + if isinstance(version, str): + pass + elif isinstance(version, (float, int)): + version = str(version) + else: + try: + version = ".".join(str(x) for x in version) + except TypeError as err: + raise RuntimeError("`version` must be a string, integer, float or an iterable") from err + return Version(version) diff --git a/tests/unit/runners/vault/test_token_auth_deprecated.py b/tests/unit/runners/vault/test_token_auth_deprecated.py index 2c50e12e..ce7209d3 100644 --- a/tests/unit/runners/vault/test_token_auth_deprecated.py +++ b/tests/unit/runners/vault/test_token_auth_deprecated.py @@ -2,7 +2,7 @@ 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. +tests/unit/runners/test_vault.py for the current tests. """ import logging from unittest.mock import ANY