Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the deprecated parameter in hvac client #31349

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions airflow/providers/hashicorp/_internal_client/vault_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@
# under the License.
from __future__ import annotations

import sys

if sys.version_info < (3, 8):
from importlib_metadata import version
else:
from importlib.metadata import version

from functools import cached_property

import hvac
Expand All @@ -30,7 +37,6 @@
DEFAULT_KUBERNETES_JWT_PATH = "/var/run/secrets/kubernetes.io/serviceaccount/token"
DEFAULT_KV_ENGINE_VERSION = 2


VALID_KV_VERSIONS: list[int] = [1, 2]
VALID_AUTH_TYPES: list[str] = [
"approle",
Expand Down Expand Up @@ -365,16 +371,25 @@ def get_secret(self, secret_path: str, secret_version: int | None = None) -> dic
:return: secret stored in the vault as a dictionary
"""
mount_point = None
hvac_version = version("hvac")
try:
mount_point, secret_path = self._parse_secret_path(secret_path)
if self.kv_engine_version == 1:
if secret_version:
raise VaultError("Secret version can only be used with version 2 of the KV engine")
response = self.client.secrets.kv.v1.read_secret(path=secret_path, mount_point=mount_point)
else:
response = self.client.secrets.kv.v2.read_secret_version(
path=secret_path, mount_point=mount_point, version=secret_version
)
if hvac_version >= "1.1.0":
response = self.client.secrets.kv.v2.read_secret_version(
path=secret_path,
mount_point=mount_point,
version=secret_version,
raise_on_deleted_version=True,
)
else:
response = self.client.secrets.kv.v2.read_secret_version(
path=secret_path, mount_point=mount_point, version=secret_version
)
except InvalidPath:
self.log.debug("Secret not found %s with mount point %s", secret_path, mount_point)
return None
Expand Down Expand Up @@ -419,11 +434,20 @@ def get_secret_including_metadata(
if self.kv_engine_version == 1:
raise VaultError("Metadata might only be used with version 2 of the KV engine.")
mount_point = None
hvac_version = version("hvac")
try:
mount_point, secret_path = self._parse_secret_path(secret_path)
return self.client.secrets.kv.v2.read_secret_version(
path=secret_path, mount_point=mount_point, version=secret_version
)
if hvac_version >= "1.1.0":
return self.client.secrets.kv.v2.read_secret_version(
path=secret_path,
mount_point=mount_point,
version=secret_version,
raise_on_deleted_version=True,
)
else:
return self.client.secrets.kv.v2.read_secret_version(
path=secret_path, mount_point=mount_point, version=secret_version
)
except InvalidPath:
self.log.debug(
"Secret not found %s with mount point %s and version %s",
Expand Down
79 changes: 61 additions & 18 deletions tests/providers/hashicorp/_internal_client/test_vault_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@
# under the License.
from __future__ import annotations

import sys

if sys.version_info < (3, 8):
from importlib_metadata import version
else:
from importlib.metadata import version

from unittest import mock
from unittest.mock import mock_open, patch

Expand Down Expand Up @@ -640,9 +647,15 @@ def test_get_non_existing_key_v2(self, mock_hvac):
)
secret = vault_client.get_secret(secret_path="missing")
assert secret is None
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None, raise_on_deleted_version=True
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None
)

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_non_existing_key_v2_different_auth(self, mock_hvac):
Expand All @@ -660,9 +673,15 @@ def test_get_non_existing_key_v2_different_auth(self, mock_hvac):
secret = vault_client.get_secret(secret_path="missing")
assert secret is None
assert "secret" == vault_client.mount_point
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None, raise_on_deleted_version=True
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None
)

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_non_existing_key_v1(self, mock_hvac):
Expand Down Expand Up @@ -715,9 +734,15 @@ def test_get_existing_key_v2(self, mock_hvac):
)
secret = vault_client.get_secret(secret_path="path/to/secret")
assert {"secret_key": "secret_value"} == secret
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="path/to/secret", version=None
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="path/to/secret", version=None, raise_on_deleted_version=True
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="path/to/secret", version=None
)

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_existing_key_v2_without_preconfigured_mount_point(self, mock_hvac):
Expand Down Expand Up @@ -753,9 +778,15 @@ def test_get_existing_key_v2_without_preconfigured_mount_point(self, mock_hvac):
)
secret = vault_client.get_secret(secret_path="mount_point/path/to/secret")
assert {"secret_key": "secret_value"} == secret
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="mount_point", path="path/to/secret", version=None
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="mount_point", path="path/to/secret", version=None, raise_on_deleted_version=True
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="mount_point", path="path/to/secret", version=None
)

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_existing_key_v2_version(self, mock_hvac):
Expand Down Expand Up @@ -790,9 +821,15 @@ def test_get_existing_key_v2_version(self, mock_hvac):
)
secret = vault_client.get_secret(secret_path="missing", secret_version=1)
assert {"secret_key": "secret_value"} == secret
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=1
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=1, raise_on_deleted_version=True
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=1
)

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_existing_key_v1(self, mock_hvac):
Expand Down Expand Up @@ -1014,9 +1051,15 @@ def test_get_secret_including_metadata_v2(self, mock_hvac):
"warnings": None,
"auth": None,
} == metadata
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None, raise_on_deleted_version=True
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None
)

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_secret_including_metadata_v1(self, mock_hvac):
Expand Down
43 changes: 34 additions & 9 deletions tests/providers/hashicorp/hooks/test_vault.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@
# under the License.
from __future__ import annotations

import sys

if sys.version_info < (3, 8):
from importlib_metadata import version
else:
from importlib.metadata import version

from unittest import mock
from unittest.mock import PropertyMock, mock_open, patch

Expand Down Expand Up @@ -1001,9 +1008,15 @@ def test_get_existing_key_v2(self, mock_hvac, mock_get_connection):
test_hook = VaultHook(**kwargs)
secret = test_hook.get_secret(secret_path="missing")
assert {"secret_key": "secret_value"} == secret
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None, raise_on_deleted_version=True
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None
)

@mock.patch("airflow.providers.hashicorp.hooks.vault.VaultHook.get_connection")
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
Expand Down Expand Up @@ -1040,9 +1053,15 @@ def test_get_existing_key_v2_version(self, mock_hvac, mock_get_connection):
test_hook = VaultHook(**kwargs)
secret = test_hook.get_secret(secret_path="missing", secret_version=1)
assert {"secret_key": "secret_value"} == secret
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=1
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=1, raise_on_deleted_version=True
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=1
)

@mock.patch("airflow.providers.hashicorp.hooks.vault.VaultHook.get_connection")
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
Expand Down Expand Up @@ -1185,9 +1204,15 @@ def test_get_secret_including_metadata_v2(self, mock_hvac, mock_get_connection):
"warnings": None,
"auth": None,
} == metadata
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None, raise_on_deleted_version=True
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="secret", path="missing", version=None
)

@mock.patch("airflow.providers.hashicorp.hooks.vault.VaultHook.get_connection")
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
Expand Down
34 changes: 28 additions & 6 deletions tests/providers/hashicorp/secrets/test_vault.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@
# under the License.
from __future__ import annotations

import sys

if sys.version_info < (3, 8):
from importlib_metadata import version
else:
from importlib.metadata import version

from unittest import mock

import pytest
Expand Down Expand Up @@ -301,9 +308,18 @@ def test_get_conn_uri_non_existent_key(self, mock_hvac):

test_client = VaultBackend(**kwargs)
assert test_client.get_conn_uri(conn_id="test_mysql") is None
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="airflow", path="connections/test_mysql", version=None
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="airflow",
path="connections/test_mysql",
version=None,
raise_on_deleted_version=True,
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="airflow", path="connections/test_mysql", version=None
)
assert test_client.get_connection(conn_id="test_mysql") is None

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
Expand Down Expand Up @@ -453,9 +469,15 @@ def test_get_variable_value_non_existent_key(self, mock_hvac):

test_client = VaultBackend(**kwargs)
assert test_client.get_variable("hello") is None
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="airflow", path="variables/hello", version=None
)
hvac_version = version("hvac")
if hvac_version >= "1.1.0":
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="airflow", path="variables/hello", version=None, raise_on_deleted_version=True
)
else:
mock_client.secrets.kv.v2.read_secret_version.assert_called_once_with(
mount_point="airflow", path="variables/hello", version=None
)
assert test_client.get_variable("hello") is None

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
Expand Down