Repository: apache/airflow
File: providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py (Python)

Code before:
            "data": {"value": "world"},
            "wrap_info": None,
            "warnings": None,
            "auth": None,
        }

        kwargs = {
            "variables_path": variables_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)
        returned_uri = test_client.get_variable(variable_key)
        mock_client.secrets.kv.v1.read_secret.assert_called_once_with(**expected_args)
        assert returned_uri == "world"

    @mock.patch.dict(
        "os.environ",
        {
            "AIRFLOW_VAR_HELLO": "world",
        },
    )
    @mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
    def test_get_variable_value_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 = {
            "variables_path": "variables",
            "mount_point": "airflow",
            "auth_type": "token",
            "url": "http://127.0.0.1:8200",
            "token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
        }

        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, raise_on_deleted_version=True
        )
        assert test_client.get_variable("hello") is None

Review comment (bug):
Since we're changing the feedback to the user regarding incorrect keys in Vault, IMO we should validate that _something_ returned to the user (in this case, a log line).  We can capture logs with [caplog](https://docs.pytest.org/en/stable/how-to/logging.html#caplog-fixture). It gets a bit lengthy, but what do you think of:

```python
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_get_variable_does_not_contain_value_key(self, mock_hvac, caplog):
    """Test that if a variable isn't stored with key 'value', a warning is logged."""
    mock_client = mock.MagicMock()
    mock_hvac.Client.return_value = mock_client
    mock_client.secrets.kv.v2.read_secret_version.return_value = {
        "request_id": "2d48a2ad-6bcb-e5b6-429d-da35fdf31f56",
        "lease_id": "",
        "renewable": False,
        "lease_duration": 0,
        "data": {
            "data": {"not_value": "world"},
            "metadata": {
                "created_time": "2020-03-28T02:10:54.301784Z",
                "deletion_time": "",
                "destroyed": False,
                "version": 1,
            },
        },
        "wrap_info": None,
        "warnings": None,
        "auth": None,
    }

    kwargs = {
        "variables_path": "variables",
        "mount_point": "airflow",
        "auth_type": "token",
        "url": "http://127.0.0.1:8200",
        "token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
    }

    test_client = VaultBackend(**kwargs)
    variable_name = "hello"
    test_client.get_variable(variable_name)

    expected_name = "airflow.providers.hashicorp.secrets.vault.VaultBackend"
    expected_levelname = "WARNING"
    expected_strings_in_msg = [variable_name, "value"]

    log_matched = False
    for record in caplog.records:
        if (
            record.name == expected_name and
            record.levelname == expected_levelname and
            all(substr in record.msg for substr in expected_strings_in_msg)
        ):
            log_matched = True
            break

    assert log_matched, "No log found warning user that key 'value' is not present in the Vault secret."
```

Code after:
        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