Skip to content
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

Feature: Support using content of kubeconfig to create KubernetesHook #39249

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

luoyuliuyin
Copy link
Contributor

@luoyuliuyin luoyuliuyin commented Apr 25, 2024

PR of Use kubeconfig as one of the optional parameters for creating KubernetesHook closes: #39227

When we use KubernetesPodOperator, we need to create KubernetesHook in order to connect to Kubernetes services. There are currently 3 supported methods for creating KubernetesHook:
1、environment variable method,
2、kubeconfig_path method,
3、db connection variable method.
image

However, these methods are all from the perspective of the airflow system owner. The airflow owner can change the variables of the airflow_worker, create and modify files in the airflow_worker, and operate the airflow_db. However, in many cases, the users of airflow are not The owner of airflow, The user does not have the authority to make changes to the airflow_worker, nor does it have read and write permissions to the db, nor should it see the data stored in the db by other users.

Therefore, it is best for users to manage their own data rather than hosting it on the airflow system, in this case, it is a relatively reasonable choice to add an optional parameter to receive the kubeconfig text.

mock_kube_config_loader.assert_called_once()
mock_kube_config_merger.assert_called_once_with("fake-temp-file")
else:
mock_tempfile.is_called_once()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no kubeconfig this function should be called?
The code under if and else blocks is the same..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function should be called.
When conn_id or kube_config exists, the content of kubeconfig file will be saved to the temporary file fake-temp-file, and then kube_config_loader will load fake-temp-file, otherwise kube_config_loader will load ~/.kube/config
I've fixed unittest and added comments.

Copy link
Collaborator

@dirrao dirrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is a one time thing, do we need to expose this?
kube config text may contain tokens. So, we need to secure them.

@luoyuliuyin
Copy link
Contributor Author

I believe this is a one time thing, do we need to expose this? kube config text may contain tokens. So, we need to secure them.

It is indeed one-time for the airflow user, but it is continuous for the airflow owner. I now manage an airflow system, and there are many users. For me, configuring the user's kubeconfig is a continuous boring job.
Now the latest version of webUI has added permission control to airflow’s dag_code, i think the security of dag_code has been guaranteed.

@luoyuliuyin
Copy link
Contributor Author

@amoghrajesh @hussein-awala @jedcunningham
Please review this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:cncf-kubernetes Kubernetes provider related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use kubeconfig as one of the optional parameters for creating KubernetesHook
3 participants