This is an automated email from the ASF dual-hosted git repository.
basph pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 57146f09588 more explicit secrets path error messages (#55015) (#59224)
57146f09588 is described below
commit 57146f09588fadfcf3363415416315485ccc8615
Author: FoxHelms <[email protected]>
AuthorDate: Tue Jan 20 08:52:19 2026 -0600
more explicit secrets path error messages (#55015) (#59224)
* more explicit secrets path error messages
* explicitly catch no value key error
* reformat invalid path message
* more explicit error getting vault config without value key
* explicit error when vault conn_uri key not present
* removing warnings from get connection
* fixing invalid path message format
* remove whitespace
* mocking logs in unit test
* simplify single use variables
* fixing log patch package import
* using back compatible package
---
airflow-core/src/airflow/models/variable.py | 2 +-
.../hashicorp/_internal_client/vault_client.py | 4 +-
.../airflow/providers/hashicorp/secrets/vault.py | 16 +++++-
.../tests/unit/hashicorp/secrets/test_vault.py | 58 ++++++++++++++++++++++
4 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/airflow-core/src/airflow/models/variable.py
b/airflow-core/src/airflow/models/variable.py
index 65808259fae..a225523b63e 100644
--- a/airflow-core/src/airflow/models/variable.py
+++ b/airflow-core/src/airflow/models/variable.py
@@ -179,7 +179,7 @@ class Variable(Base, LoggingMixin):
if var_val is None:
if default_var is not cls.__NO_DEFAULT_SENTINEL:
return default_var
- raise KeyError(f"Variable {key} does not exist")
+ raise KeyError(f"Variable {key} does not exist.")
if deserialize_json:
obj = json.loads(var_val)
mask_secret(obj, key)
diff --git
a/providers/hashicorp/src/airflow/providers/hashicorp/_internal_client/vault_client.py
b/providers/hashicorp/src/airflow/providers/hashicorp/_internal_client/vault_client.py
index aa73ffdc8b7..e1e63d788c2 100644
---
a/providers/hashicorp/src/airflow/providers/hashicorp/_internal_client/vault_client.py
+++
b/providers/hashicorp/src/airflow/providers/hashicorp/_internal_client/vault_client.py
@@ -421,7 +421,9 @@ class _VaultClient(LoggingMixin):
if not self.mount_point:
split_secret_path = secret_path.split("/", 1)
if len(split_secret_path) < 2:
- raise InvalidPath
+ raise InvalidPath(
+ "The variable path you have provided is invalid. Please
provide a full path of the format: path/to/secret/variable"
+ )
return split_secret_path[0], split_secret_path[1]
return self.mount_point, secret_path
diff --git
a/providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py
b/providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py
index ad5cdc7c21f..40812a44830 100644
--- a/providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py
+++ b/providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py
@@ -226,7 +226,13 @@ class VaultBackend(BaseSecretsBackend, LoggingMixin):
response = self.vault_client.get_secret(
secret_path=(mount_point + "/" if mount_point else "") +
secret_path
)
- return response.get("value") if response else None
+ if not response:
+ return None
+ try:
+ return response["value"]
+ except KeyError:
+ self.log.warning('Vault secret %s fetched but does not have
required key "value"', key)
+ return None
def get_config(self, key: str) -> str | None:
"""
@@ -245,4 +251,10 @@ class VaultBackend(BaseSecretsBackend, LoggingMixin):
response = self.vault_client.get_secret(
secret_path=(mount_point + "/" if mount_point else "") +
secret_path
)
- return response.get("value") if response else None
+ if not response:
+ return None
+ try:
+ return response["value"]
+ except KeyError:
+ self.log.warning('Vault config %s fetched but does not have
required key "value"', key)
+ return None
diff --git a/providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py
b/providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py
index 6cc248304d6..667e7859450 100644
--- a/providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py
+++ b/providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py
@@ -263,6 +263,64 @@ class TestVaultSecrets:
)
assert test_client.get_variable("hello") is None
+
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
+ @mock.patch("airflow.utils.log.logging_mixin.logging")
+ def test_get_variable_does_not_contain_value_key(self, mock_hvac,
mock_get_logger):
+ """
+ Test that if the 'value' key is not present in Vault,
_VaultClient.get_variable
+ should log a warning and return None
+ """
+ mock_hvac.Client.return_value = mock.MagicMock()
+ mock_logger = mock.MagicMock()
+ mock_get_logger.getLogger.return_value = mock_logger
+
+ test_client = VaultBackend(
+ **{
+ "variables_path": "variables",
+ "mount_point": "airflow",
+ "auth_type": "token",
+ "url": "http://127.0.0.1:8200",
+ "token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
+ }
+ )
+ test_client._log = mock_logger
+ test_client.vault_client.get_secret =
mock.MagicMock(return_value={"test_key": "data"})
+ result = test_client.get_variable("hello")
+ assert result is None
+ mock_logger.warning.assert_called_with(
+ 'Vault secret %s fetched but does not have required key "value"',
"hello"
+ )
+
+
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
+ @mock.patch("airflow.utils.log.logging_mixin.logging")
+ def test_get_config_does_not_contain_value_key(self, mock_hvac,
mock_get_logger):
+ """
+ Test that if the 'value' key is not present in Vault,
_VaultClient.get_config
+ should log a warning and return None
+ """
+ mock_client = mock.MagicMock()
+ mock_hvac.Client.return_value = mock_client
+ mock_logger = mock.MagicMock()
+ mock_get_logger.getLogger.return_value = mock_logger
+
+ kwargs = {
+ "variables_path": "variables",
+ "mount_point": "airflow",
+ "auth_type": "token",
+ "url": "http://127.0.0.1:8200",
+ "token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
+ }
+ test_client = VaultBackend(**kwargs)
+ test_client._log = mock_logger
+ response = {"test_key": "data"}
+ test_client.vault_client.get_secret =
mock.MagicMock(return_value=response)
+
+ returned_uri = test_client.get_config("sql_alchemy_conn")
+ assert returned_uri is None
+ mock_logger.warning.assert_called_with(
+ 'Vault config %s fetched but does not have required key "value"',
"sql_alchemy_conn"
+ )
+
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_auth_failure_raises_error(self, mock_hvac):
mock_client = mock.MagicMock()