-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
fix: mount kerberos.keytab to worker when enable #29526
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/main/CONTRIBUTING.rst)
|
Can you please add tests covering that one @npsables ? I think if you add tests, it will be immediately visible what effect it will have. |
Hi sorry for late reply. I observed that my keytab cannot be found (in worker pods) when enabling Kerberos in helm, it doesn't make sense at all. People may want to use it for other services. If you still think this is a good idea, I can create a testcase (check if kerberos.keytab exists after render) for this PR. |
Yes. Adding tests is useful to show that. We generally expect all the changes to be covered by tests. So showing tests especially ones that have assertions showing the behaviour you observed (and failing because of that if your change is not implemented) is exactly that is needed. |
Hi, I added a testcase. Also I notice this function name is misspelled (keberos). Do you want me to change it too? airflow/tests/charts/test_kerberos.py Line 94 in 5ad2ed1
|
Static checks need fixing. |
All good now |
Awesome work, congrats on your first merged pull request! |
Indeed. |
This is actually the first time I have contributed to any opensource. It's meaningful to me. Thanks a lot! |
Cool. Maybe you would like to take part in this campaign? https://news.apache.org/foundation/entry/the-asf-launches-firstasfcontribution-campaign - fill-in a short form and the ASF marketing team will contact you if they would like to get more insight :) |
I believe kerberos.keytab and others should be mounted to worker container when enabling kerberos. We may want to reuse these files, for example, to spark-submit jobs. Maybe #24369 is a good reference for this pr.