From 0c9da502f203b1d7ea84de4eb801453663dd3a96 Mon Sep 17 00:00:00 2001 From: Daniel Standish <15932138+dstandish@users.noreply.github.com> Date: Thu, 18 Nov 2021 16:41:53 -0800 Subject: [PATCH 1/9] Add config and context params to KubernetesHook This is mainly needed in order to enable use KubernetesHook with KubernetesPodOperator while maintaining backw ard compatibility. --- .../cncf/kubernetes/hooks/kubernetes.py | 47 ++++- .../cncf/kubernetes/hooks/test_kubernetes.py | 164 +++++++++++------- 2 files changed, 144 insertions(+), 67 deletions(-) diff --git a/airflow/providers/cncf/kubernetes/hooks/kubernetes.py b/airflow/providers/cncf/kubernetes/hooks/kubernetes.py index f2e3d4d3fe5d1..81452f20dd349 100644 --- a/airflow/providers/cncf/kubernetes/hooks/kubernetes.py +++ b/airflow/providers/cncf/kubernetes/hooks/kubernetes.py @@ -85,6 +85,9 @@ def get_connection_form_widgets() -> Dict[str, Any]: "extra__kubernetes__namespace": StringField( lazy_gettext('Namespace'), widget=BS3TextFieldWidget() ), + "extra__kubernetes__cluster_context": StringField( + lazy_gettext('Cluster context'), widget=BS3TextFieldWidget() + ), } @staticmethod @@ -96,25 +99,46 @@ def get_ui_field_behaviour() -> Dict: } def __init__( - self, conn_id: str = default_conn_name, client_configuration: Optional[client.Configuration] = None + self, + conn_id: str = default_conn_name, + client_configuration: Optional[client.Configuration] = None, + cluster_context: Optional[str] = None, + config_file: Optional[str] = None, + in_cluster: Optional[bool] = None, ) -> None: super().__init__() self.conn_id = conn_id self.client_configuration = client_configuration + self.cluster_context = cluster_context + self.config_file = config_file + self.in_cluster = in_cluster + + @staticmethod + def _coalesce_param(*params): + for param in params: + if param is not None: + return param def get_conn(self) -> Any: """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") or None - kubeconfig_path = extras.get("extra__kubernetes__kube_config_path") or None + in_cluster = self._coalesce_param( + self.in_cluster, extras.get("extra__kubernetes__in_cluster") or None + ) + cluster_context = self._coalesce_param( + self.cluster_context, extras.get("extra__kubernetes__cluster_context") or None + ) + kubeconfig_path = self._coalesce_param( + self.config_file, 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: raise AirflowException( - "Invalid connection configuration. Options extra__kubernetes__kube_config_path, " - "extra__kubernetes__kube_config, extra__kubernetes__in_cluster are mutually exclusive. " + "Invalid connection configuration. Options kube_config_path, " + "kube_config, in_cluster are mutually exclusive. " "You can only use one option at a time." ) if in_cluster: @@ -125,7 +149,9 @@ def get_conn(self) -> Any: if kubeconfig_path is not None: self.log.debug("loading kube_config from: %s", kubeconfig_path) config.load_kube_config( - config_file=kubeconfig_path, client_configuration=self.client_configuration + config_file=kubeconfig_path, + client_configuration=self.client_configuration, + context=cluster_context, ) return client.ApiClient() @@ -135,12 +161,17 @@ def get_conn(self) -> Any: temp_config.write(kubeconfig.encode()) temp_config.flush() config.load_kube_config( - config_file=temp_config.name, client_configuration=self.client_configuration + config_file=temp_config.name, + client_configuration=self.client_configuration, + context=cluster_context, ) return client.ApiClient() self.log.debug("loading kube_config from: default file") - config.load_kube_config(client_configuration=self.client_configuration) + config.load_kube_config( + client_configuration=self.client_configuration, + context=cluster_context, + ) return client.ApiClient() @cached_property diff --git a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py index 31e81d8029609..c51a2f014379a 100644 --- a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py +++ b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py @@ -20,13 +20,11 @@ import json import os import tempfile -import unittest from unittest import mock from unittest.mock import patch import kubernetes import pytest -from parameterized import parameterized from airflow import AirflowException from airflow.models import Connection @@ -37,107 +35,155 @@ KUBE_CONFIG_PATH = os.getenv('KUBECONFIG', '~/.kube/config') -class TestKubernetesHook(unittest.TestCase): +class TestKubernetesHook: @classmethod - def setUpClass(cls) -> None: + def setup_class(cls) -> None: 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', {}), + ('in_cluster', {'extra__kubernetes__in_cluster': True}), + ('kube_config', {'extra__kubernetes__kube_config': '{"test": "kube"}'}), + ('kube_config_path', {'extra__kubernetes__kube_config_path': 'path/to/file'}), + ('in_cluster_empty', {'extra__kubernetes__in_cluster': ''}), + ('kube_config_empty', {'extra__kubernetes__kube_config': ''}), + ('kube_config_path_empty', {'extra__kubernetes__kube_config_path': ''}), + ('kube_config_empty', {'extra__kubernetes__kube_config': ''}), + ('kube_config_path_empty', {'extra__kubernetes__kube_config_path': ''}), + ('context_empty', {'extra__kubernetes__cluster_context': ''}), + ('context', {'extra__kubernetes__cluster_context': 'my-context'}), + ('with_namespace', {'extra__kubernetes__namespace': 'mock_namespace'}), + ('default_kube_config', {}), ]: db.merge_conn(Connection(conn_type='kubernetes', conn_id=conn_id, extra=json.dumps(extra))) @classmethod - def tearDownClass(cls) -> None: + def teardown_class(cls) -> None: clear_db_connections() - @patch("kubernetes.config.incluster_config.InClusterConfigLoader") - def test_in_cluster_connection(self, mock_kube_config_loader): - kubernetes_hook = KubernetesHook(conn_id='kubernetes_in_cluster') - api_conn = kubernetes_hook.get_conn() - 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) - + @pytest.mark.parametrize( + 'in_cluster_param, conn_id, in_cluster_called', + ( + pytest.param(None, 'in_cluster', True), + pytest.param(True, 'in_cluster', True), + pytest.param(False, 'in_cluster', False), + pytest.param(None, 'in_cluster_empty', False), + pytest.param(True, 'in_cluster_empty', True), + pytest.param(False, 'in_cluster_empty', False), + ), + ) @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): - kubernetes_hook = KubernetesHook(conn_id='kubernetes_kube_config_path') + @patch("kubernetes.config.incluster_config.InClusterConfigLoader") + def test_in_cluster_connection( + self, + mock_in_cluster_loader, + mock_merger, + mock_loader, + in_cluster_param, + conn_id, + in_cluster_called, + ): + kubernetes_hook = KubernetesHook(conn_id=conn_id, in_cluster=in_cluster_param) api_conn = kubernetes_hook.get_conn() - mock_kube_config_loader.assert_called_once_with("path/to/file") - mock_kube_config_merger.assert_called_once() + if in_cluster_called: + mock_in_cluster_loader.assert_called_once() + mock_merger.assert_not_called() + mock_loader.assert_not_called() + else: + mock_in_cluster_loader.assert_not_called() + mock_merger.assert_called_once_with(KUBE_CONFIG_PATH) + mock_loader.assert_called_once() assert isinstance(api_conn, kubernetes.client.api_client.ApiClient) + @pytest.mark.parametrize( + 'config_path_param, conn_id, call_path', + ( + pytest.param(None, 'kube_config_path', 'path/to/file'), + pytest.param('/my/path/override', 'kube_config_path', '/my/path/override'), + pytest.param(None, 'kube_config_path_empty', KUBE_CONFIG_PATH), + pytest.param('/my/path/override', 'kube_config_path_empty', '/my/path/override'), + ), + ) @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') + def test_kube_config_path( + self, mock_kube_config_merger, mock_kube_config_loader, config_path_param, conn_id, call_path + ): + kubernetes_hook = KubernetesHook(conn_id=conn_id, config_file=config_path_param) api_conn = kubernetes_hook.get_conn() - mock_kube_config_loader.assert_called_once_with(KUBE_CONFIG_PATH) - mock_kube_config_merger.assert_called_once() + mock_kube_config_merger.assert_called_once_with(call_path) + mock_kube_config_loader.assert_called_once() assert isinstance(api_conn, kubernetes.client.api_client.ApiClient) + @pytest.mark.parametrize( + 'conn_id, has_config', + ( + pytest.param('kube_config', True), + pytest.param('kube_config_empty', False), + ), + ) @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') + def test_kube_config_connection( + self, mock_tempfile, mock_kube_config_merger, mock_kube_config_loader, conn_id, has_config + ): + mock_tempfile.return_value.__enter__.return_value.name = "fake-temp-file" + mock_kube_config_merger.return_value.config = {"fake_config": "value"} + kubernetes_hook = KubernetesHook(conn_id=conn_id) api_conn = kubernetes_hook.get_conn() - mock_tempfile.is_called_once() - mock_kube_config_loader.assert_called_once() - mock_kube_config_merger.assert_called_once() + if has_config: + mock_tempfile.is_called_once() + mock_kube_config_loader.assert_called_once() + mock_kube_config_merger.assert_called_once_with('fake-temp-file') + else: + mock_tempfile.assert_not_called() + mock_kube_config_loader.assert_called_once() + mock_kube_config_merger.assert_called_once_with(KUBE_CONFIG_PATH) 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_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) + @pytest.mark.parametrize( + 'context_param, conn_id, expected_context', + ( + pytest.param('param-context', 'context', 'param-context'), + pytest.param(None, 'context', 'my-context'), + pytest.param('param-context', 'context_empty', 'param-context'), + pytest.param(None, 'context_empty', None), + ), + ) + @patch("kubernetes.config.load_kube_config") + def test_cluster_context(self, mock_load_kube_config, context_param, conn_id, expected_context): + kubernetes_hook = KubernetesHook(conn_id=conn_id, cluster_context=context_param) + kubernetes_hook.get_conn() + mock_load_kube_config.assert_called_with(client_configuration=None, context=expected_context) @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, - mock_kube_config_loader, mock_kube_config_merger, + mock_kube_config_loader, ): - kubernetes_hook = KubernetesHook(conn_id='kubernetes_default_kube_config') + kubernetes_hook = KubernetesHook(conn_id='default_kube_config') api_conn = kubernetes_hook.get_conn() - mock_kube_config_loader.assert_called_once_with("/mock/config") - mock_kube_config_merger.assert_called_once() + mock_kube_config_merger.assert_called_once_with("/mock/config") + mock_kube_config_loader.assert_called_once() assert isinstance(api_conn, kubernetes.client.api_client.ApiClient) def test_get_namespace(self): - kubernetes_hook_with_namespace = KubernetesHook(conn_id='kubernetes_with_namespace') - kubernetes_hook_without_namespace = KubernetesHook(conn_id='kubernetes_default_kube_config') + kubernetes_hook_with_namespace = KubernetesHook(conn_id='with_namespace') + kubernetes_hook_without_namespace = KubernetesHook(conn_id='default_kube_config') assert kubernetes_hook_with_namespace.get_namespace() == 'mock_namespace' assert kubernetes_hook_without_namespace.get_namespace() == 'default' -class TestKubernetesHookIncorrectConfiguration(unittest.TestCase): - @parameterized.expand( +class TestKubernetesHookIncorrectConfiguration: + @pytest.mark.parametrize( + 'conn_uri', ( "kubernetes://?extra__kubernetes__kube_config_path=/tmp/&extra__kubernetes__kube_config=[1,2,3]", "kubernetes://?extra__kubernetes__kube_config_path=/tmp/&extra__kubernetes__in_cluster=[1,2,3]", "kubernetes://?extra__kubernetes__kube_config=/tmp/&extra__kubernetes__in_cluster=[1,2,3]", - ) + ), ) def test_should_raise_exception_on_invalid_configuration(self, conn_uri): with mock.patch.dict("os.environ", AIRFLOW_CONN_KUBERNETES_DEFAULT=conn_uri), pytest.raises( From fd556f23851e3a1768ad361cbaab1d417a195cdd Mon Sep 17 00:00:00 2001 From: Daniel Standish <15932138+dstandish@users.noreply.github.com> Date: Fri, 19 Nov 2021 14:42:49 -0800 Subject: [PATCH 2/9] add test docstrings --- .../cncf/kubernetes/hooks/test_kubernetes.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py index c51a2f014379a..0cf1355cf3613 100644 --- a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py +++ b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py @@ -81,6 +81,10 @@ def test_in_cluster_connection( conn_id, in_cluster_called, ): + """ + Verifies whether in_cluster is called depending on combination of hook param and connection extra. + Hook param should beat extra. + """ kubernetes_hook = KubernetesHook(conn_id=conn_id, in_cluster=in_cluster_param) api_conn = kubernetes_hook.get_conn() if in_cluster_called: @@ -107,6 +111,10 @@ def test_in_cluster_connection( def test_kube_config_path( self, mock_kube_config_merger, mock_kube_config_loader, config_path_param, conn_id, call_path ): + """ + Verifies kube config path depending on combination of hook param and connection extra. + Hook param should beat extra. + """ kubernetes_hook = KubernetesHook(conn_id=conn_id, config_file=config_path_param) api_conn = kubernetes_hook.get_conn() mock_kube_config_merger.assert_called_once_with(call_path) @@ -126,6 +134,9 @@ def test_kube_config_path( def test_kube_config_connection( self, mock_tempfile, mock_kube_config_merger, mock_kube_config_loader, conn_id, has_config ): + """ + Verifies whether temporary kube config file is created. + """ mock_tempfile.return_value.__enter__.return_value.name = "fake-temp-file" mock_kube_config_merger.return_value.config = {"fake_config": "value"} kubernetes_hook = KubernetesHook(conn_id=conn_id) @@ -151,6 +162,10 @@ def test_kube_config_connection( ) @patch("kubernetes.config.load_kube_config") def test_cluster_context(self, mock_load_kube_config, context_param, conn_id, expected_context): + """ + Verifies cluster context depending on combination of hook param and connection extra. + Hook param should beat extra. + """ kubernetes_hook = KubernetesHook(conn_id=conn_id, cluster_context=context_param) kubernetes_hook.get_conn() mock_load_kube_config.assert_called_with(client_configuration=None, context=expected_context) From 02091a5e39aa773fa30c9f3d7a0b53e92fcb74f4 Mon Sep 17 00:00:00 2001 From: Daniel Standish <15932138+dstandish@users.noreply.github.com> Date: Mon, 22 Nov 2021 11:21:38 -0800 Subject: [PATCH 3/9] add tests for no connection and client type --- .../cncf/kubernetes/hooks/kubernetes.py | 22 +++++++++++++------ .../cncf/kubernetes/hooks/test_kubernetes.py | 21 +++++++++++++----- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/airflow/providers/cncf/kubernetes/hooks/kubernetes.py b/airflow/providers/cncf/kubernetes/hooks/kubernetes.py index 81452f20dd349..e7c558c5b4c6e 100644 --- a/airflow/providers/cncf/kubernetes/hooks/kubernetes.py +++ b/airflow/providers/cncf/kubernetes/hooks/kubernetes.py @@ -100,7 +100,7 @@ def get_ui_field_behaviour() -> Dict: def __init__( self, - conn_id: str = default_conn_name, + conn_id: Optional[str] = default_conn_name, client_configuration: Optional[client.Configuration] = None, cluster_context: Optional[str] = None, config_file: Optional[str] = None, @@ -121,8 +121,11 @@ def _coalesce_param(*params): def get_conn(self) -> Any: """Returns kubernetes api session for use with requests""" - connection = self.get_connection(self.conn_id) - extras = connection.extra_dejson + if self.conn_id: + connection = self.get_connection(self.conn_id) + extras = connection.extra_dejson + else: + extras = {} in_cluster = self._coalesce_param( self.in_cluster, extras.get("extra__kubernetes__in_cluster") or None ) @@ -179,6 +182,10 @@ def api_client(self) -> Any: """Cached Kubernetes API client""" return self.get_conn() + @cached_property + def core_v1_client(self): + return client.CoreV1Api(api_client=self.api_client) + def create_custom_object( self, group: str, version: str, plural: str, body: Union[str, dict], namespace: Optional[str] = None ): @@ -240,10 +247,11 @@ def get_custom_object( def get_namespace(self) -> str: """Returns the namespace that defined in the connection""" - connection = self.get_connection(self.conn_id) - extras = connection.extra_dejson - namespace = extras.get("extra__kubernetes__namespace", "default") - return namespace + if self.conn_id: + connection = self.get_connection(self.conn_id) + extras = connection.extra_dejson + namespace = extras.get("extra__kubernetes__namespace", "default") + return namespace def get_pod_log_stream( self, diff --git a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py index 0cf1355cf3613..360f837c93d51 100644 --- a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py +++ b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py @@ -61,6 +61,8 @@ def teardown_class(cls) -> None: @pytest.mark.parametrize( 'in_cluster_param, conn_id, in_cluster_called', ( + pytest.param(True, None, True), + pytest.param(None, None, False), pytest.param(None, 'in_cluster', True), pytest.param(True, 'in_cluster', True), pytest.param(False, 'in_cluster', False), @@ -100,6 +102,8 @@ def test_in_cluster_connection( @pytest.mark.parametrize( 'config_path_param, conn_id, call_path', ( + pytest.param(None, None, KUBE_CONFIG_PATH), + pytest.param('/my/path/override', None, '/my/path/override'), pytest.param(None, 'kube_config_path', 'path/to/file'), pytest.param('/my/path/override', 'kube_config_path', '/my/path/override'), pytest.param(None, 'kube_config_path_empty', KUBE_CONFIG_PATH), @@ -124,6 +128,7 @@ def test_kube_config_path( @pytest.mark.parametrize( 'conn_id, has_config', ( + pytest.param(None, False), pytest.param('kube_config', True), pytest.param('kube_config_empty', False), ), @@ -154,6 +159,8 @@ def test_kube_config_connection( @pytest.mark.parametrize( 'context_param, conn_id, expected_context', ( + pytest.param('param-context', None, 'param-context'), + pytest.param(None, None, None), pytest.param('param-context', 'context', 'param-context'), pytest.param(None, 'context', 'my-context'), pytest.param('param-context', 'context_empty', 'param-context'), @@ -173,11 +180,7 @@ def test_cluster_context(self, mock_load_kube_config, context_param, conn_id, ex @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, - mock_kube_config_merger, - mock_kube_config_loader, - ): + def test_default_kube_config_connection(self, mock_kube_config_merger, mock_kube_config_loader): kubernetes_hook = KubernetesHook(conn_id='default_kube_config') api_conn = kubernetes_hook.get_conn() mock_kube_config_merger.assert_called_once_with("/mock/config") @@ -190,6 +193,14 @@ def test_get_namespace(self): assert kubernetes_hook_with_namespace.get_namespace() == 'mock_namespace' assert kubernetes_hook_without_namespace.get_namespace() == 'default' + @patch("kubernetes.config.kube_config.KubeConfigLoader") + @patch("kubernetes.config.kube_config.KubeConfigMerger") + def test_client_types(self, mock_kube_config_merger, mock_kube_config_loader): + hook = KubernetesHook(None) + assert isinstance(hook.core_v1_client, kubernetes.client.CoreV1Api) + assert isinstance(hook.api_client, kubernetes.client.ApiClient) + assert isinstance(hook.get_conn(), kubernetes.client.ApiClient) + class TestKubernetesHookIncorrectConfiguration: @pytest.mark.parametrize( From 649a4fcbdd2565ae9bcdead43b6482a6a3b57f41 Mon Sep 17 00:00:00 2001 From: Daniel Standish <15932138+dstandish@users.noreply.github.com> Date: Wed, 1 Dec 2021 13:06:16 -0800 Subject: [PATCH 4/9] Update airflow/providers/cncf/kubernetes/hooks/kubernetes.py Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com> --- airflow/providers/cncf/kubernetes/hooks/kubernetes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow/providers/cncf/kubernetes/hooks/kubernetes.py b/airflow/providers/cncf/kubernetes/hooks/kubernetes.py index e7c558c5b4c6e..aa784b2293641 100644 --- a/airflow/providers/cncf/kubernetes/hooks/kubernetes.py +++ b/airflow/providers/cncf/kubernetes/hooks/kubernetes.py @@ -245,7 +245,7 @@ def get_custom_object( except client.rest.ApiException as e: raise AirflowException(f"Exception when calling -> get_custom_object: {e}\n") - def get_namespace(self) -> str: + def get_namespace(self) -> Optional[str]: """Returns the namespace that defined in the connection""" if self.conn_id: connection = self.get_connection(self.conn_id) From 0cf525cfcec28b65a7e0ce62753d71b92f7d3871 Mon Sep 17 00:00:00 2001 From: Daniel Standish <15932138+dstandish@users.noreply.github.com> Date: Wed, 1 Dec 2021 14:55:52 -0800 Subject: [PATCH 5/9] Update tests/providers/cncf/kubernetes/hooks/test_kubernetes.py Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com> --- .../cncf/kubernetes/hooks/test_kubernetes.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py index 360f837c93d51..b78052302a3bd 100644 --- a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py +++ b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py @@ -61,14 +61,14 @@ def teardown_class(cls) -> None: @pytest.mark.parametrize( 'in_cluster_param, conn_id, in_cluster_called', ( - pytest.param(True, None, True), - pytest.param(None, None, False), - pytest.param(None, 'in_cluster', True), - pytest.param(True, 'in_cluster', True), - pytest.param(False, 'in_cluster', False), - pytest.param(None, 'in_cluster_empty', False), - pytest.param(True, 'in_cluster_empty', True), - pytest.param(False, 'in_cluster_empty', False), + (True, None, True), + (None, None, False), + (None, 'in_cluster', True), + (True, 'in_cluster', True), + (False, 'in_cluster', False), + (None, 'in_cluster_empty', False), + (True, 'in_cluster_empty', True), + (False, 'in_cluster_empty', False), ), ) @patch("kubernetes.config.kube_config.KubeConfigLoader") From 209b26d25f0ee893db9c7a993dc6e20c7d4b7ea9 Mon Sep 17 00:00:00 2001 From: Daniel Standish <15932138+dstandish@users.noreply.github.com> Date: Wed, 1 Dec 2021 15:00:11 -0800 Subject: [PATCH 6/9] Update tests/providers/cncf/kubernetes/hooks/test_kubernetes.py Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com> --- .../cncf/kubernetes/hooks/test_kubernetes.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py index b78052302a3bd..1d0eb15f6cc31 100644 --- a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py +++ b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py @@ -102,12 +102,12 @@ def test_in_cluster_connection( @pytest.mark.parametrize( 'config_path_param, conn_id, call_path', ( - pytest.param(None, None, KUBE_CONFIG_PATH), - pytest.param('/my/path/override', None, '/my/path/override'), - pytest.param(None, 'kube_config_path', 'path/to/file'), - pytest.param('/my/path/override', 'kube_config_path', '/my/path/override'), - pytest.param(None, 'kube_config_path_empty', KUBE_CONFIG_PATH), - pytest.param('/my/path/override', 'kube_config_path_empty', '/my/path/override'), + (None, None, KUBE_CONFIG_PATH), + ('/my/path/override', None, '/my/path/override'), + (None, 'kube_config_path', 'path/to/file'), + ('/my/path/override', 'kube_config_path', '/my/path/override'), + (None, 'kube_config_path_empty', KUBE_CONFIG_PATH), + ('/my/path/override', 'kube_config_path_empty', '/my/path/override'), ), ) @patch("kubernetes.config.kube_config.KubeConfigLoader") From 3277a50716bf619421adba286a2c31b14d9349aa Mon Sep 17 00:00:00 2001 From: Daniel Standish <15932138+dstandish@users.noreply.github.com> Date: Wed, 1 Dec 2021 15:00:31 -0800 Subject: [PATCH 7/9] Update tests/providers/cncf/kubernetes/hooks/test_kubernetes.py Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com> --- tests/providers/cncf/kubernetes/hooks/test_kubernetes.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py index 1d0eb15f6cc31..da120391c4a87 100644 --- a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py +++ b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py @@ -128,9 +128,9 @@ def test_kube_config_path( @pytest.mark.parametrize( 'conn_id, has_config', ( - pytest.param(None, False), - pytest.param('kube_config', True), - pytest.param('kube_config_empty', False), + (None, False), + ('kube_config', True), + ('kube_config_empty', False), ), ) @patch("kubernetes.config.kube_config.KubeConfigLoader") From 85468d69c67c150ee44a46f00d7b91c1a839e486 Mon Sep 17 00:00:00 2001 From: Daniel Standish <15932138+dstandish@users.noreply.github.com> Date: Wed, 1 Dec 2021 15:00:43 -0800 Subject: [PATCH 8/9] Update tests/providers/cncf/kubernetes/hooks/test_kubernetes.py Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com> --- .../cncf/kubernetes/hooks/test_kubernetes.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py index da120391c4a87..4670c6812f364 100644 --- a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py +++ b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py @@ -159,12 +159,12 @@ def test_kube_config_connection( @pytest.mark.parametrize( 'context_param, conn_id, expected_context', ( - pytest.param('param-context', None, 'param-context'), - pytest.param(None, None, None), - pytest.param('param-context', 'context', 'param-context'), - pytest.param(None, 'context', 'my-context'), - pytest.param('param-context', 'context_empty', 'param-context'), - pytest.param(None, 'context_empty', None), + ('param-context', None, 'param-context'), + (None, None, None), + ('param-context', 'context', 'param-context'), + (None, 'context', 'my-context'), + ('param-context', 'context_empty', 'param-context'), + (None, 'context_empty', None), ), ) @patch("kubernetes.config.load_kube_config") From ef99068ac3694e57e5f9f800e872c6c3fc1a4c93 Mon Sep 17 00:00:00 2001 From: Daniel Standish <15932138+dstandish@users.noreply.github.com> Date: Wed, 1 Dec 2021 15:16:40 -0800 Subject: [PATCH 9/9] add tests for namespace --- .../cncf/kubernetes/hooks/test_kubernetes.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py index 4670c6812f364..256974ebd40d8 100644 --- a/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py +++ b/tests/providers/cncf/kubernetes/hooks/test_kubernetes.py @@ -63,6 +63,7 @@ def teardown_class(cls) -> None: ( (True, None, True), (None, None, False), + (False, None, False), (None, 'in_cluster', True), (True, 'in_cluster', True), (False, 'in_cluster', False), @@ -187,11 +188,17 @@ def test_default_kube_config_connection(self, mock_kube_config_merger, mock_kube mock_kube_config_loader.assert_called_once() assert isinstance(api_conn, kubernetes.client.api_client.ApiClient) - def test_get_namespace(self): - kubernetes_hook_with_namespace = KubernetesHook(conn_id='with_namespace') - kubernetes_hook_without_namespace = KubernetesHook(conn_id='default_kube_config') - assert kubernetes_hook_with_namespace.get_namespace() == 'mock_namespace' - assert kubernetes_hook_without_namespace.get_namespace() == 'default' + @pytest.mark.parametrize( + 'conn_id, expected', + ( + pytest.param(None, None, id='no-conn-id'), + pytest.param('with_namespace', 'mock_namespace', id='conn-with-namespace'), + pytest.param('default_kube_config', 'default', id='conn-without-namespace'), + ), + ) + def test_get_namespace(self, conn_id, expected): + hook = KubernetesHook(conn_id=conn_id) + assert hook.get_namespace() == expected @patch("kubernetes.config.kube_config.KubeConfigLoader") @patch("kubernetes.config.kube_config.KubeConfigMerger")