Skip to content

Commit

Permalink
Removed deprecated code from hashicorp provider (apache#44598)
Browse files Browse the repository at this point in the history 
* Remove deprecated code from HashiCorp provider

* Remove additional deprecated code from hashicorp  provider

* remove test code for deprecated method

* fix if clause for approle authentication

* modified changelog

---------

Co-authored-by: pratiksha rajendrabhai badheka <pratiksha@DESKTOP-T5HUA05>
  • Loading branch information
@Prab-27
2 people authored and Lefteris Gilmaz committed Jan 5, 2025
1 parent a89e7b5 commit 46de067
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 257 deletions.
12 changes: 12 additions & 0 deletions providers/src/airflow/providers/hashicorp/CHANGELOG.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@
Changelog
---------

main
.....

.. warning::
All deprecated classes, parameters and features have been removed from the hashicorp provider package.
The following breaking changes were introduced:

* The usage of role_id for AppRole authentication has been deprecated from airflow.providers.hashicorp.hook.vault .Please use connection login
* The usage of role_id in connection extra for AppRole authentication has been deprecated from airflow.providers.hashicorp.hook.vault. Please use connection login
* Removed role_id from get_connection_form_widgets
* Removed deprecated method ``VaultBackend.get_conn_uri`` from airflow.providers.hashicorp.secrets.vault

3.8.0
.....

Expand Down
25 changes: 3 additions & 22 deletions providers/src/airflow/providers/hashicorp/hooks/vault.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@
from __future__ import annotations

import json
import warnings
from typing import TYPE_CHECKING, Any

from hvac.exceptions import VaultError

from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.hooks.base import BaseHook
from airflow.providers.hashicorp._internal_client.vault_client import (
DEFAULT_KUBERNETES_JWT_PATH,
Expand Down Expand Up @@ -70,7 +68,7 @@ class VaultHook(BaseHook):
Login/Password are used as credentials:
* approle: login -> role_id, password -> secret_id
* approle: login -> connection.login
* github: password -> token
* token: password -> token
* aws_iam: login -> key_id, password -> secret_id
Expand Down Expand Up @@ -147,24 +145,8 @@ def __init__(
if kwargs:
client_kwargs = merge_dicts(client_kwargs, kwargs)

if auth_type == "approle":
if role_id:
warnings.warn(
"""The usage of role_id for AppRole authentication has been deprecated.
Please use connection login.""",
AirflowProviderDeprecationWarning,
stacklevel=2,
)
elif self.connection.extra_dejson.get("role_id"):
role_id = self.connection.extra_dejson.get("role_id")
warnings.warn(
"""The usage of role_id in connection extra for AppRole authentication has been
deprecated. Please use connection login.""",
AirflowProviderDeprecationWarning,
stacklevel=2,
)
elif self.connection.login:
role_id = self.connection.login
if auth_type == "approle" and self.connection.login:
role_id = self.connection.login

if auth_type == "aws_iam":
if not role_id:
Expand Down Expand Up @@ -385,7 +367,6 @@ def get_connection_form_widgets(cls) -> dict[str, Any]:
description="Must be 1 or 2.",
default=DEFAULT_KV_ENGINE_VERSION,
),
"role_id": StringField(lazy_gettext("Role ID (deprecated)"), widget=BS3TextFieldWidget()),
"kubernetes_role": StringField(lazy_gettext("Kubernetes role"), widget=BS3TextFieldWidget()),
"kubernetes_jwt_path": StringField(
lazy_gettext("Kubernetes jwt path"), widget=BS3TextFieldWidget()
Expand Down
19 changes: 0 additions & 19 deletions providers/src/airflow/providers/hashicorp/secrets/vault.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@

from typing import TYPE_CHECKING

from deprecated import deprecated

from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.providers.hashicorp._internal_client.vault_client import _VaultClient
from airflow.secrets import BaseSecretsBackend
from airflow.utils.log.logging_mixin import LoggingMixin
Expand Down Expand Up @@ -191,22 +188,6 @@ def get_response(self, conn_id: str) -> dict | None:
secret_path=(mount_point + "/" if mount_point else "") + secret_path
)

@deprecated(
reason="Method `VaultBackend.get_conn_uri` is deprecated and will be removed in a future release.",
category=AirflowProviderDeprecationWarning,
)
def get_conn_uri(self, conn_id: str) -> str | None:
"""
Get serialized representation of connection.
:param conn_id: The connection id
:return: The connection uri retrieved from the secret
"""
# Since VaultBackend implements `get_connection`, `get_conn_uri` is not used. So we
# don't need to implement (or direct users to use) method `get_conn_value` instead
response = self.get_response(conn_id)
return response.get("conn_uri") if response else None

# Make sure connection is imported this way for type checking, otherwise when importing
# the backend it will get a circular dependency and fail
if TYPE_CHECKING:
Expand Down
216 changes: 0 additions & 216 deletions providers/tests/hashicorp/secrets/test_vault.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,89 +21,10 @@
import pytest
from hvac.exceptions import InvalidPath, VaultError

from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.providers.hashicorp.secrets.vault import VaultBackend


class TestVaultSecrets:
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_conn_uri(self, mock_hvac):
mock_client = mock.MagicMock()
mock_hvac.Client.return_value = mock_client
mock_client.secrets.kv.v2.read_secret_version.return_value = {
"request_id": "94011e25-f8dc-ec29-221b-1f9c1d9ad2ae",
"lease_id": "",
"renewable": False,
"lease_duration": 0,
"data": {
"data": {"conn_uri": "postgresql://airflow:airflow@host:5432/airflow"},
"metadata": {
"created_time": "2020-03-16T21:01:43.331126Z",
"deletion_time": "",
"destroyed": False,
"version": 1,
},
},
"wrap_info": None,
"warnings": None,
"auth": None,
}

kwargs = {
"connections_path": "connections",
"mount_point": "airflow",
"auth_type": "token",
"url": "http://127.0.0.1:8200",
"token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
}

test_client = VaultBackend(**kwargs)
with pytest.warns(
AirflowProviderDeprecationWarning,
match="Method `VaultBackend.get_conn_uri` is deprecated and will be removed in a future release.",
):
returned_uri = test_client.get_conn_uri(conn_id="test_postgres")
assert returned_uri == "postgresql://airflow:airflow@host:5432/airflow"

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_conn_uri_without_predefined_mount_point(self, mock_hvac):
mock_client = mock.MagicMock()
mock_hvac.Client.return_value = mock_client
mock_client.secrets.kv.v2.read_secret_version.return_value = {
"request_id": "94011e25-f8dc-ec29-221b-1f9c1d9ad2ae",
"lease_id": "",
"renewable": False,
"lease_duration": 0,
"data": {
"data": {"conn_uri": "postgresql://airflow:airflow@host:5432/airflow"},
"metadata": {
"created_time": "2020-03-16T21:01:43.331126Z",
"deletion_time": "",
"destroyed": False,
"version": 1,
},
},
"wrap_info": None,
"warnings": None,
"auth": None,
}

kwargs = {
"connections_path": "connections",
"mount_point": None,
"auth_type": "token",
"url": "http://127.0.0.1:8200",
"token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
}

test_client = VaultBackend(**kwargs)
with pytest.warns(
AirflowProviderDeprecationWarning,
match="Method `VaultBackend.get_conn_uri` is deprecated and will be removed in a future release.",
):
returned_uri = test_client.get_conn_uri(conn_id="airflow/test_postgres")
assert returned_uri == "postgresql://airflow:airflow@host:5432/airflow"

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_connection(self, mock_hvac):
mock_client = mock.MagicMock()
Expand Down Expand Up @@ -190,143 +111,6 @@ def test_get_connection_without_predefined_mount_point(self, mock_hvac):
connection = test_client.get_connection(conn_id="airflow/test_postgres")
assert connection.get_uri() == "postgresql://airflow:airflow@host:5432/airflow?foo=bar&baz=taz"

@pytest.mark.parametrize(
"mount_point, connections_path, conn_id, expected_args",
[
(
"airflow",
"connections",
"test_postgres",
{"mount_point": "airflow", "path": "connections/test_postgres"},
),
(
"airflow",
"",
"path/to/connections/test_postgres",
{"mount_point": "airflow", "path": "path/to/connections/test_postgres"},
),
(
None,
"connections",
"airflow/test_postgres",
{"mount_point": "airflow", "path": "connections/test_postgres"},
),
(
None,
"",
"airflow/path/to/connections/test_postgres",
{"mount_point": "airflow", "path": "path/to/connections/test_postgres"},
),
],
)
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_conn_uri_engine_version_1(
self, mock_hvac, mount_point, connections_path, conn_id, expected_args
):
mock_client = mock.MagicMock()
mock_hvac.Client.return_value = mock_client
mock_client.secrets.kv.v1.read_secret.return_value = {
"request_id": "182d0673-618c-9889-4cba-4e1f4cfe4b4b",
"lease_id": "",
"renewable": False,
"lease_duration": 2764800,
"data": {"conn_uri": "postgresql://airflow:airflow@host:5432/airflow"},
"wrap_info": None,
"warnings": None,
"auth": None,
}

kwargs = {
"connections_path": connections_path,
"mount_point": mount_point,
"auth_type": "token",
"url": "http://127.0.0.1:8200",
"token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
"kv_engine_version": 1,
}

test_client = VaultBackend(**kwargs)
with pytest.warns(
AirflowProviderDeprecationWarning,
match="Method `VaultBackend.get_conn_uri` is deprecated and will be removed in a future release.",
):
returned_uri = test_client.get_conn_uri(conn_id=conn_id)
mock_client.secrets.kv.v1.read_secret.assert_called_once_with(**expected_args)
assert returned_uri == "postgresql://airflow:airflow@host:5432/airflow"

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_conn_uri_engine_version_1_custom_auth_mount_point(self, mock_hvac):
mock_client = mock.MagicMock()
mock_hvac.Client.return_value = mock_client
mock_client.secrets.kv.v1.read_secret.return_value = {
"request_id": "182d0673-618c-9889-4cba-4e1f4cfe4b4b",
"lease_id": "",
"renewable": False,
"lease_duration": 2764800,
"data": {"conn_uri": "postgresql://airflow:airflow@host:5432/airflow"},
"wrap_info": None,
"warnings": None,
"auth": None,
}

kwargs = {
"connections_path": "connections",
"mount_point": "airflow",
"auth_mount_point": "custom",
"auth_type": "token",
"url": "http://127.0.0.1:8200",
"token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
"kv_engine_version": 1,
}

test_client = VaultBackend(**kwargs)
assert test_client.vault_client.auth_mount_point == "custom"
with pytest.warns(
AirflowProviderDeprecationWarning,
match="Method `VaultBackend.get_conn_uri` is deprecated and will be removed in a future release.",
):
returned_uri = test_client.get_conn_uri(conn_id="test_postgres")
mock_client.secrets.kv.v1.read_secret.assert_called_once_with(
mount_point="airflow", path="connections/test_postgres"
)
assert returned_uri == "postgresql://airflow:airflow@host:5432/airflow"

@mock.patch.dict(
"os.environ",
{
"AIRFLOW_CONN_TEST_MYSQL": "mysql://airflow:airflow@host:5432/airflow",
},
)
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_conn_uri_non_existent_key(self, mock_hvac):
"""
Test that if the key with connection ID is not present in Vault, _VaultClient.get_connection
should return None
"""
mock_client = mock.MagicMock()
mock_hvac.Client.return_value = mock_client
# Response does not contain the requested key
mock_client.secrets.kv.v2.read_secret_version.side_effect = InvalidPath()

kwargs = {
"connections_path": "connections",
"mount_point": "airflow",
"auth_type": "token",
"url": "http://127.0.0.1:8200",
"token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
}

test_client = VaultBackend(**kwargs)
with pytest.warns(
AirflowProviderDeprecationWarning,
match="Method `VaultBackend.get_conn_uri` is deprecated and will be removed in a future release.",
):
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, raise_on_deleted_version=True
)
assert test_client.get_connection(conn_id="test_mysql") is None

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_variable_value(self, mock_hvac):
mock_client = mock.MagicMock()
Expand Down

0 comments on commit 46de067

Please sign in to comment.