-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support k8s auth method in vault secrets provider #8640
Conversation
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 Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
|
cf040d0
to
e43696c
Compare
e43696c
to
752e371
Compare
Adds rudimentary support for authenticating to vault using the kubernetes auth method.
752e371
to
e57f9f8
Compare
Not sure about the most recent lint failure...I moved pylint directive comment to the line above the class. |
You can see it in the output: airflow/providers/hashicorp/secrets/vault.py:32:54: E261 at least two spaces before inline comment |
But in the most recent code there is no inline comment at |
Pushed an empty commit to try again. Thanks for feedback :). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested this ?
Yes, for the most part. See this gist for an example of an adhoc end-to-end test that works: https://gist.github.com/reltuk/4ff3ee7edca0848d081a945abb5ad01b |
Small white lie from the above testing: in order to make it work, I had to have the following patch applied. Not sure what's going on there...maybe my pip install is wrong or diff --git a/airflow/providers/hashicorp/secrets/vault.py b/airflow/providers/hashicorp/secrets/vault.py
index c0fd23581..27058e9c4 100644
--- a/airflow/providers/hashicorp/secrets/vault.py
+++ b/airflow/providers/hashicorp/secrets/vault.py
@@ -24,7 +24,7 @@ import hvac
from cached_property import cached_property
from hvac.exceptions import InvalidPath, VaultError
-from airflow import AirflowException
+from airflow.exceptions import AirflowException
from airflow.secrets import BaseSecretsBackend
from airflow.utils.log.logging_mixin import LoggingMixin
Seems unrelated to the k8s support though. |
Happy for you to make that change too. It will avoid cycles. |
Ok, added a commit to fix the AirflowException import in Also added a commit to make |
Awesome work, congrats on your first merged pull request! |
(cherry picked from commit d8cb0b5)
(cherry picked from commit d8cb0b5)
(cherry picked from commit d8cb0b5)
(cherry picked from commit d8cb0b5)
Make sure to mark the boxes below before creating PR: [x]
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.