Skip to content

Commit 39b4804

Browse files
authored
Avoid extra_dejson in ADF and Synapse async hooks (#72130)
extra_dejson can mask secrets via a sync send on the triggerer event loop, which raises AsyncToSync. Parse extras with json.loads instead, matching the MSGraph workaround. closes: #55728
1 parent 82215cf commit 39b4804

4 files changed

Lines changed: 117 additions & 9 deletions

File tree

providers/microsoft/azure/src/airflow/providers/microsoft/azure/hooks/data_factory.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
from __future__ import annotations
3535

3636
import inspect
37+
import json
3738
import time
3839
from collections.abc import Callable
3940
from functools import wraps
@@ -1124,7 +1125,8 @@ async def bind_argument(arg: Any, default_key: str) -> None:
11241125
if arg not in bound_args.arguments or bound_args.arguments[arg] is None:
11251126
self = args[0]
11261127
conn = await get_async_connection(self.conn_id)
1127-
extras = conn.extra_dejson
1128+
# extra_dejson can call mask_secret -> sync send on the triggerer loop.
1129+
extras = json.loads(conn.extra) if conn.extra else {}
11281130
default_value = extras.get(default_key) or extras.get(
11291131
f"extra__azure_data_factory__{default_key}"
11301132
)
@@ -1175,7 +1177,8 @@ async def get_async_conn(self) -> AsyncDataFactoryManagementClient:
11751177
return self._async_conn
11761178

11771179
conn = await get_async_connection(self.conn_id)
1178-
extras = conn.extra_dejson
1180+
# extra_dejson can call mask_secret -> sync send on the triggerer loop.
1181+
extras = json.loads(conn.extra) if conn.extra else {}
11791182
tenant = get_field(extras, "tenantId")
11801183

11811184
try:
@@ -1208,6 +1211,7 @@ async def get_async_conn(self) -> AsyncDataFactoryManagementClient:
12081211

12091212
async def refresh_conn(self) -> AsyncDataFactoryManagementClient: # type: ignore[override]
12101213
self._conn = None
1214+
await self.close()
12111215
return await self.get_async_conn()
12121216

12131217
@provide_targeted_factory_async

providers/microsoft/azure/src/airflow/providers/microsoft/azure/hooks/synapse.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
# under the License.
1717
from __future__ import annotations
1818

19+
import json
1920
import time
2021
from typing import TYPE_CHECKING, Any
2122

@@ -29,6 +30,7 @@
2930
from azure.synapse.artifacts.aio import ArtifactsClient as AsyncArtifactsClient
3031
from azure.synapse.spark import SparkClient
3132

33+
from airflow.providers.common.compat.connection import get_async_connection
3234
from airflow.providers.common.compat.sdk import AirflowException, AirflowTaskTimeout, BaseHook
3335
from airflow.providers.microsoft.azure.utils import (
3436
add_managed_identity_connection_widgets,
@@ -489,8 +491,9 @@ async def get_async_conn(self) -> AsyncArtifactsClient:
489491
if self._async_conn is not None:
490492
return self._async_conn
491493

492-
conn = self.get_connection(self.conn_id)
493-
extras = conn.extra_dejson
494+
conn = await get_async_connection(self.conn_id)
495+
# extra_dejson can call mask_secret -> sync send on the triggerer loop.
496+
extras = json.loads(conn.extra) if conn.extra else {}
494497
tenant = self._get_field(extras, "tenantId")
495498

496499
credential: AsyncCredentials

providers/microsoft/azure/tests/unit/microsoft/azure/hooks/test_data_factory.py

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
# under the License.
1717
from __future__ import annotations
1818

19+
import json
1920
import os
2021
from unittest import mock
2122
from unittest.mock import MagicMock, PropertyMock, patch
@@ -716,20 +717,31 @@ async def test_get_adf_pipeline_run_status_cancelled(self, mock_get_pipeline_run
716717
assert response == mock_status
717718

718719
@pytest.mark.asyncio
719-
@mock.patch(f"{MODULE}.AzureDataFactoryAsyncHook.get_connection")
720+
@mock.patch(f"{MODULE}.get_async_connection", new_callable=mock.AsyncMock)
720721
@mock.patch(f"{MODULE}.AzureDataFactoryAsyncHook.get_async_conn")
721-
async def test_get_pipeline_run_exception_without_resource(self, mock_conn, mock_get_connection):
722+
async def test_get_pipeline_run_exception_without_resource(self, mock_conn, mock_get_async_connection):
722723
"""
723724
Test get_pipeline_run function without passing the resource name to check the decorator function and
724725
raise exception
725726
"""
726727
mock_connection = Connection(extra={"factory_name": DATAFACTORY_NAME})
727-
mock_get_connection.return_value = mock_connection
728+
mock_get_async_connection.return_value = mock_connection
728729
mock_conn.return_value.pipeline_runs.get.return_value = MagicMock()
729730
hook = AzureDataFactoryAsyncHook(AZURE_DATA_FACTORY_CONN_ID)
730731
with pytest.raises(AirflowException):
731732
await hook.get_pipeline_run(RUN_ID, None, DATAFACTORY_NAME)
732733

734+
@staticmethod
735+
def _conn_with_raising_extra_dejson(extra: dict, login="clientId", password="clientSecret"):
736+
conn = mock.Mock()
737+
conn.login = login
738+
conn.password = password
739+
conn.extra = json.dumps(extra)
740+
type(conn).extra_dejson = PropertyMock(
741+
side_effect=RuntimeError("You cannot use AsyncToSync in the same thread as an async event loop")
742+
)
743+
return conn
744+
733745
@pytest.mark.asyncio
734746
@pytest.mark.parametrize(
735747
"mocked_connection",
@@ -786,6 +798,50 @@ async def test_get_async_conn(self, mocked_connection):
786798
response = await hook.get_async_conn()
787799
assert isinstance(response, DataFactoryManagementClient)
788800

801+
@pytest.mark.asyncio
802+
async def test_get_async_conn_does_not_touch_extra_dejson(self):
803+
conn = self._conn_with_raising_extra_dejson(
804+
{"tenantId": "tenantId", "subscriptionId": "subscriptionId"}
805+
)
806+
hook = AzureDataFactoryAsyncHook(AZURE_DATA_FACTORY_CONN_ID)
807+
with (
808+
mock.patch(f"{MODULE}.get_async_connection", new=mock.AsyncMock(return_value=conn)),
809+
mock.patch(f"{MODULE}.AsyncClientSecretCredential"),
810+
mock.patch(f"{MODULE}.AsyncDataFactoryManagementClient") as mock_client,
811+
):
812+
response = await hook.get_async_conn()
813+
assert response is mock_client.return_value
814+
815+
@pytest.mark.asyncio
816+
async def test_get_async_conn_uses_get_async_connection(self):
817+
conn = self._conn_with_raising_extra_dejson(
818+
{"tenantId": "tenantId", "subscriptionId": "subscriptionId"}
819+
)
820+
hook = AzureDataFactoryAsyncHook(AZURE_DATA_FACTORY_CONN_ID)
821+
with (
822+
mock.patch(
823+
f"{MODULE}.get_async_connection", new=mock.AsyncMock(return_value=conn)
824+
) as mock_get_async_connection,
825+
mock.patch(f"{MODULE}.AsyncClientSecretCredential"),
826+
mock.patch(f"{MODULE}.AsyncDataFactoryManagementClient"),
827+
):
828+
await hook.get_async_conn()
829+
mock_get_async_connection.assert_awaited_once_with(AZURE_DATA_FACTORY_CONN_ID)
830+
831+
@pytest.mark.asyncio
832+
@mock.patch(f"{MODULE}.AzureDataFactoryAsyncHook.get_async_conn")
833+
async def test_provide_targeted_factory_async_does_not_touch_extra_dejson(self, mock_get_async_conn):
834+
conn = self._conn_with_raising_extra_dejson(
835+
{"resource_group_name": RESOURCE_GROUP_NAME, "factory_name": DATAFACTORY_NAME}
836+
)
837+
mock_get_async_conn.return_value.pipeline_runs.get = mock.AsyncMock(return_value=MagicMock())
838+
hook = AzureDataFactoryAsyncHook(AZURE_DATA_FACTORY_CONN_ID)
839+
with mock.patch(f"{MODULE}.get_async_connection", new=mock.AsyncMock(return_value=conn)):
840+
await hook.get_pipeline_run(RUN_ID, None, None)
841+
mock_get_async_conn.return_value.pipeline_runs.get.assert_awaited_once_with(
842+
RESOURCE_GROUP_NAME, DATAFACTORY_NAME, RUN_ID
843+
)
844+
789845
@pytest.mark.asyncio
790846
@pytest.mark.parametrize(
791847
"mocked_connection",
@@ -904,10 +960,14 @@ def test_get_field_non_prefixed_extras(self):
904960
@pytest.mark.asyncio
905961
@mock.patch(f"{MODULE}.AzureDataFactoryAsyncHook.get_async_conn")
906962
async def test_refresh_conn(self, mock_get_async_conn):
907-
"""Test refresh_conn method _conn is reset and get_async_conn is called"""
963+
"""Test refresh_conn closes the async client before recreating it."""
908964
hook = AzureDataFactoryAsyncHook(AZURE_DATA_FACTORY_CONN_ID)
965+
mock_async_conn = mock.AsyncMock()
966+
hook._async_conn = mock_async_conn
909967
await hook.refresh_conn()
910968
assert not hook._conn
969+
mock_async_conn.close.assert_awaited_once()
970+
assert hook._async_conn is None
911971
assert mock_get_async_conn.called
912972

913973
@pytest.mark.asyncio

providers/microsoft/azure/tests/unit/microsoft/azure/hooks/test_synapse_pipeline.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
# under the License.
1717
from __future__ import annotations
1818

19-
from unittest.mock import AsyncMock, MagicMock, patch
19+
import json
20+
from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch
2021

2122
import pytest
2223
from azure.synapse.artifacts import ArtifactsClient
@@ -247,6 +248,46 @@ async def test_get_async_conn_client_secret(self, mock_credential, mock_client):
247248
credential=mock_credential.return_value,
248249
)
249250

251+
@pytest.mark.asyncio
252+
@patch(f"{MODULE}.AsyncArtifactsClient")
253+
@patch(f"{MODULE}.AsyncClientSecretCredential")
254+
async def test_get_async_conn_does_not_touch_extra_dejson(self, mock_credential, mock_client):
255+
conn = MagicMock()
256+
conn.login = "clientId"
257+
conn.password = "clientSecret"
258+
conn.extra = json.dumps({"tenantId": "tenantId"})
259+
type(conn).extra_dejson = PropertyMock(
260+
side_effect=RuntimeError("You cannot use AsyncToSync in the same thread as an async event loop")
261+
)
262+
hook = AzureSynapsePipelineAsyncHook(
263+
azure_synapse_conn_id=DEFAULT_CONNECTION_CLIENT_SECRET,
264+
azure_synapse_workspace_dev_endpoint=AZURE_SYNAPSE_WORKSPACE_DEV_ENDPOINT,
265+
)
266+
with patch(f"{MODULE}.get_async_connection", new=AsyncMock(return_value=conn)):
267+
result = await hook.get_async_conn()
268+
assert result is mock_client.return_value
269+
mock_credential.assert_called_with(
270+
client_id="clientId",
271+
client_secret="clientSecret",
272+
tenant_id="tenantId",
273+
)
274+
275+
@pytest.mark.asyncio
276+
@patch(f"{MODULE}.AsyncArtifactsClient")
277+
@patch(f"{MODULE}.AsyncClientSecretCredential")
278+
async def test_get_async_conn_uses_get_async_connection(self, mock_credential, mock_client):
279+
conn = MagicMock()
280+
conn.login = "clientId"
281+
conn.password = "clientSecret"
282+
conn.extra = json.dumps({"tenantId": "tenantId"})
283+
hook = AzureSynapsePipelineAsyncHook(
284+
azure_synapse_conn_id=DEFAULT_CONNECTION_CLIENT_SECRET,
285+
azure_synapse_workspace_dev_endpoint=AZURE_SYNAPSE_WORKSPACE_DEV_ENDPOINT,
286+
)
287+
with patch(f"{MODULE}.get_async_connection", new=AsyncMock(return_value=conn)) as mock_get:
288+
await hook.get_async_conn()
289+
mock_get.assert_awaited_once_with(DEFAULT_CONNECTION_CLIENT_SECRET)
290+
250291
@pytest.mark.asyncio
251292
@patch(f"{MODULE}.AsyncArtifactsClient")
252293
@patch(f"{MODULE}.AsyncDefaultAzureCredential")

0 commit comments

Comments
 (0)