You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by ds...@apache.org on 2021/11/19 16:01:48 UTC
[airflow] branch main updated: Coalesce `extra` params to None in KubernetesHook (#19694)
This is an automated email from the ASF dual-hosted git repository.
dstandish pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new f7410df Coalesce `extra` params to None in KubernetesHook (#19694)
f7410df is described below
commit f7410dfba268c6b6bbb7832a13c547a6d98afabe
Author: Daniel Standish <15...@users.noreply.github.com>
AuthorDate: Fri Nov 19 08:01:13 2021 -0800
Coalesce `extra` params to None in KubernetesHook (#19694)
Coalesce `extra` params to None in KubernetesHook
When using UI form widgets FAB provides a empty string by default for every param. This turns out to make a difference sometimes. E.g. in this hook, we decide what to do depending on whether the param `is not None` -- and if you've created the connection in the UI, even though you didn't supply a value for this param, it will be `not None`
In this case, I do not think this is a breaking change because e.g. if `kubeconfig_path` is empty string then loading it should fail. This should just allow better functioning of the hook.
---
.../providers/cncf/kubernetes/hooks/kubernetes.py | 6 +-
.../cncf/kubernetes/hooks/test_kubernetes.py | 72 ++++++++++++----------
2 files changed, 44 insertions(+), 34 deletions(-)
diff --git a/airflow/providers/cncf/kubernetes/hooks/kubernetes.py b/airflow/providers/cncf/kubernetes/hooks/kubernetes.py
index e230dba..f2e3d4d 100644
--- a/airflow/providers/cncf/kubernetes/hooks/kubernetes.py
+++ b/airflow/providers/cncf/kubernetes/hooks/kubernetes.py
@@ -106,9 +106,9 @@ class KubernetesHook(BaseHook):
"""Returns kubernetes api session for use with requests"""
connection = self.get_connection(self.conn_id)
extras = connection.extra_dejson
- in_cluster = extras.get("extra__kubernetes__in_cluster")
- kubeconfig_path = extras.get("extra__kubernetes__kube_config_path")
- kubeconfig = extras.get("extra__kubernetes__kube_config")
+ in_cluster = extras.get("extra__kubernetes__in_cluster") or None
+ kubeconfig_path = extras.get("extra__kubernetes__kube_config_path") or None
+ kubeconfig = extras.get("extra__kubernetes__kube_config") or None
num_selected_configuration = len([o for o in [in_cluster, kubeconfig, kubeconfig_path] if o])
if num_selected_configuration > 1:
diff --git a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py
index c8c2de4..31e81d8 100644
--- a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py
+++ b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py
@@ -18,6 +18,7 @@
#
import json
+import os
import tempfile
import unittest
from unittest import mock
@@ -33,41 +34,23 @@ from airflow.providers.cncf.kubernetes.hooks.kubernetes import KubernetesHook
from airflow.utils import db
from tests.test_utils.db import clear_db_connections
+KUBE_CONFIG_PATH = os.getenv('KUBECONFIG', '~/.kube/config')
+
class TestKubernetesHook(unittest.TestCase):
@classmethod
def setUpClass(cls) -> None:
- db.merge_conn(
- Connection(
- conn_id='kubernetes_in_cluster',
- conn_type='kubernetes',
- extra=json.dumps({'extra__kubernetes__in_cluster': True}),
- )
- )
- db.merge_conn(
- Connection(
- conn_id='kubernetes_kube_config',
- conn_type='kubernetes',
- extra=json.dumps({'extra__kubernetes__kube_config': '{"test": "kube"}'}),
- )
- )
- db.merge_conn(
- Connection(
- conn_id='kubernetes_kube_config_path',
- conn_type='kubernetes',
- extra=json.dumps({'extra__kubernetes__kube_config_path': 'path/to/file'}),
- )
- )
- db.merge_conn(
- Connection(conn_id='kubernetes_default_kube_config', conn_type='kubernetes', extra=json.dumps({}))
- )
- db.merge_conn(
- Connection(
- conn_id='kubernetes_with_namespace',
- conn_type='kubernetes',
- extra=json.dumps({'extra__kubernetes__namespace': 'mock_namespace'}),
- )
- )
+ for conn_id, extra in [
+ ('kubernetes_in_cluster', {'extra__kubernetes__in_cluster': True}),
+ ('kubernetes_kube_config', {'extra__kubernetes__kube_config': '{"test": "kube"}'}),
+ ('kubernetes_kube_config_path', {'extra__kubernetes__kube_config_path': 'path/to/file'}),
+ ('kubernetes_in_cluster_empty', {'extra__kubernetes__in_cluster': ''}),
+ ('kubernetes_kube_config_empty', {'extra__kubernetes__kube_config': ''}),
+ ('kubernetes_kube_config_path_empty', {'extra__kubernetes__kube_config_path': ''}),
+ ('kubernetes_with_namespace', {'extra__kubernetes__namespace': 'mock_namespace'}),
+ ('kubernetes_default_kube_config', {}),
+ ]:
+ db.merge_conn(Connection(conn_type='kubernetes', conn_id=conn_id, extra=json.dumps(extra)))
@classmethod
def tearDownClass(cls) -> None:
@@ -80,6 +63,15 @@ class TestKubernetesHook(unittest.TestCase):
mock_kube_config_loader.assert_called_once()
assert isinstance(api_conn, kubernetes.client.api_client.ApiClient)
+ @patch("kubernetes.config.kube_config.KubeConfigMerger")
+ @patch("kubernetes.config.kube_config.KubeConfigLoader")
+ def test_in_cluster_connection_empty(self, mock_kube_config_merger, mock_kube_config_loader):
+ kubernetes_hook = KubernetesHook(conn_id='kubernetes_in_cluster_empty')
+ api_conn = kubernetes_hook.get_conn()
+ mock_kube_config_loader.assert_called_once_with(KUBE_CONFIG_PATH)
+ mock_kube_config_merger.assert_called_once()
+ assert isinstance(api_conn, kubernetes.client.api_client.ApiClient)
+
@patch("kubernetes.config.kube_config.KubeConfigLoader")
@patch("kubernetes.config.kube_config.KubeConfigMerger")
def test_kube_config_path(self, mock_kube_config_loader, mock_kube_config_merger):
@@ -91,6 +83,15 @@ class TestKubernetesHook(unittest.TestCase):
@patch("kubernetes.config.kube_config.KubeConfigLoader")
@patch("kubernetes.config.kube_config.KubeConfigMerger")
+ def test_kube_config_path_empty(self, mock_kube_config_loader, mock_kube_config_merger):
+ kubernetes_hook = KubernetesHook(conn_id='kubernetes_kube_config_path_empty')
+ api_conn = kubernetes_hook.get_conn()
+ mock_kube_config_loader.assert_called_once_with(KUBE_CONFIG_PATH)
+ mock_kube_config_merger.assert_called_once()
+ assert isinstance(api_conn, kubernetes.client.api_client.ApiClient)
+
+ @patch("kubernetes.config.kube_config.KubeConfigLoader")
+ @patch("kubernetes.config.kube_config.KubeConfigMerger")
@patch.object(tempfile, 'NamedTemporaryFile')
def test_kube_config_connection(self, mock_kube_config_loader, mock_kube_config_merger, mock_tempfile):
kubernetes_hook = KubernetesHook(conn_id='kubernetes_kube_config')
@@ -102,6 +103,15 @@ class TestKubernetesHook(unittest.TestCase):
@patch("kubernetes.config.kube_config.KubeConfigLoader")
@patch("kubernetes.config.kube_config.KubeConfigMerger")
+ def test_kube_config_connection_empty(self, mock_kube_config_loader, mock_kube_config_merger):
+ kubernetes_hook = KubernetesHook(conn_id='kubernetes_kube_config_empty')
+ api_conn = kubernetes_hook.get_conn()
+ mock_kube_config_loader.assert_called_once_with(KUBE_CONFIG_PATH)
+ mock_kube_config_merger.assert_called_once()
+ assert isinstance(api_conn, kubernetes.client.api_client.ApiClient)
+
+ @patch("kubernetes.config.kube_config.KubeConfigLoader")
+ @patch("kubernetes.config.kube_config.KubeConfigMerger")
@patch("kubernetes.config.kube_config.KUBE_CONFIG_DEFAULT_LOCATION", "/mock/config")
def test_default_kube_config_connection(
self,