inherit key "verify" from env into session so that kv_client can read it properly

[chz8494]

---

Currently Hashicorp Vault as secret backend against private vault doesn't work and facing ssl CA trust issue.
Example config:

[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultBackend
backend_kwargs = {"connections_path": "airflow-connections", "variables_path": null, "mount_point": "airflow", "url": "http://127.0.0.1:8200", "verify": "/etc/ssl/certs/ca-certificates"}

current code ignores verify value and causing CA trust failure.
I did some debug and found that the dep kv client tends to inherit session keys, so I just added verify value into session.

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

• Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
• In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
• Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
• Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: dev@airflow.apache.org
  Slack: https://s.apache.org/airflow-slack

[hussein-awala]

The verify argument is present in Airflow documentation , but looks like it was accidentally removed by a change, so we need to add a test to avoid removing your fix in the future; could you add the test?
https://github.com/apache/airflow/blob/3568b09e8c7501bd84d08d594038dea9b8e20a23/tests/providers/hashicorp/_internal_client/test_vault_client.py

[chz8494]

The verify argument is present in Airflow documentation , but looks like it was accidentally removed by a change, so we need to add a test to avoid removing your fix in the future; could you add the test? https://github.com/apache/airflow/blob/3568b09e8c7501bd84d08d594038dea9b8e20a23/tests/providers/hashicorp/_internal_client/test_vault_client.py

airflow/tests/providers/hashicorp/_internal_client/test_vault_client.py

Line 615 in d3dc88f

def test_userpass(self, mock_hvac):

I'd like to help but don't understand how to run this test, seems it needs to be used against local vault server localhost:8081? but I don't know how it can be used on git-ci...
In addition, I traced back up to 2.1.0 but couldn't find any code related to verify, I'd assume this session change was new in recent kv_client update and verify logic in provider was never in place, someone coded it and expected **kwargs could automatically cover verify key value, which it's not happening.

see kv client doc and code:
https://github.com/hvac/hvac/blob/48027db42c037fe8f6b1c6fd8f6dbf80c1ea8595/docs/advanced_usage.rst#L108
https://github.com/hvac/hvac/blob/48027db42c037fe8f6b1c6fd8f6dbf80c1ea8595/hvac/adapters.py#L90

[hussein-awala]

I'd like to help but don't understand how to run this test, seems it needs to be used against local vault server localhost:8081?

They are unit tests and not integration tests, so we use Python mock package to patch the methods/classes.
You can check the testing docs in the contributing documentation.

In addition, I traced back up to 2.1.0 but couldn't find any code related to verify, I'd assume this session change was new in recent kv_client update and verify logic in provider was never in place, someone coded it and expected **kwargs could automatically cover verify key value, which it's not happening.

This is possible, but we need to add tests to avoid breaking the fix in the future.

[chz8494]

@hussein-awala not sure if I'm doing it correctly, just added ssl case for v1 test. Also noticed a corner case problem with condition check, so I also change code to be:

if self.kwargs and "verify" in self.kwargs:
                session.verify = self.kwargs["verify"]

[chz8494]

@hussein-awala can someone review and suggest if tests good?

[eladkal]

Static tests are failing. Can you look into it?

[chz8494]

Static tests are failing. Can you look into it?

fixed

[chz8494]

@hussein-awala @eladkal can someone help merge it? I'd like to see it works in the new release

[boring-cyborg]

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

[hussein-awala]

Congrats on your first commit 🎉

[melicheradam]

@hussein-awala Hello, any updates on when this will get released? Since it was merged on 15th may but was not included in the 2.9.2 version

[potiuk]

@hussein-awala Hello, any updates on when this will get released? Since it was merged on 15th may but was not included in the 2.9.2 version

It's provider change - look at provider's changelogs and releases. What do you make of it @melicheradam ?

[melicheradam]

@potiuk thanks, I was not aware that providers have their separate changelogs and releases :)