From 1c5bf586f0882c81c03181588830887345703ca5 Mon Sep 17 00:00:00 2001 From: April Schleck Date: Thu, 23 Dec 2021 14:46:23 -0800 Subject: [PATCH 1/2] Run kubeconfig exec commands in the correct directory. This fixes configs that rely on relative paths. --- config/exec_provider.py | 4 +++- config/exec_provider_test.py | 21 +++++++++++++++------ config/kube_config.py | 4 ++-- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/config/exec_provider.py b/config/exec_provider.py index 4008f2e8b..ef3fac661 100644 --- a/config/exec_provider.py +++ b/config/exec_provider.py @@ -31,7 +31,7 @@ class ExecProvider(object): * caching """ - def __init__(self, exec_config): + def __init__(self, exec_config, cwd): """ exec_config must be of type ConfigNode because we depend on safe_get(self, key) to correctly handle optional exec provider @@ -53,6 +53,7 @@ class ExecProvider(object): value = item['value'] additional_vars[name] = value self.env.update(additional_vars) + self.cwd = cwd def run(self, previous_response=None): kubernetes_exec_info = { @@ -69,6 +70,7 @@ class ExecProvider(object): self.args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + cwd=self.cwd, env=self.env, universal_newlines=True) (stdout, stderr) = process.communicate() diff --git a/config/exec_provider_test.py b/config/exec_provider_test.py index 44579beb2..a545b5565 100644 --- a/config/exec_provider_test.py +++ b/config/exec_provider_test.py @@ -47,7 +47,7 @@ class ExecProviderTest(unittest.TestCase): ConfigNode('test3', {'apiVersion': ''})] for exec_config in exec_configs: with self.assertRaises(ConfigException) as context: - ExecProvider(exec_config) + ExecProvider(exec_config, None) self.assertIn('exec: malformed request. missing key', context.exception.args[0]) @@ -57,7 +57,7 @@ class ExecProviderTest(unittest.TestCase): instance.wait.return_value = 1 instance.communicate.return_value = ('', '') with self.assertRaises(ConfigException) as context: - ep = ExecProvider(self.input_ok) + ep = ExecProvider(self.input_ok, None) ep.run() self.assertIn('exec: process returned %d' % instance.wait.return_value, context.exception.args[0]) @@ -68,7 +68,7 @@ class ExecProviderTest(unittest.TestCase): instance.wait.return_value = 0 instance.communicate.return_value = ('', '') with self.assertRaises(ConfigException) as context: - ep = ExecProvider(self.input_ok) + ep = ExecProvider(self.input_ok, None) ep.run() self.assertIn('exec: failed to decode process output', context.exception.args[0]) @@ -102,7 +102,7 @@ class ExecProviderTest(unittest.TestCase): for output in outputs: instance.communicate.return_value = (output, '') with self.assertRaises(ConfigException) as context: - ep = ExecProvider(self.input_ok) + ep = ExecProvider(self.input_ok, None) ep.run() self.assertIn('exec: malformed response. missing key', context.exception.args[0]) @@ -123,7 +123,7 @@ class ExecProviderTest(unittest.TestCase): """ % wrong_api_version instance.communicate.return_value = (output, '') with self.assertRaises(ConfigException) as context: - ep = ExecProvider(self.input_ok) + ep = ExecProvider(self.input_ok, None) ep.run() self.assertIn( 'exec: plugin api version %s does not match' % @@ -135,11 +135,20 @@ class ExecProviderTest(unittest.TestCase): instance = mock.return_value instance.wait.return_value = 0 instance.communicate.return_value = (self.output_ok, '') - ep = ExecProvider(self.input_ok) + ep = ExecProvider(self.input_ok, None) result = ep.run() self.assertTrue(isinstance(result, dict)) self.assertTrue('token' in result) + @mock.patch('subprocess.Popen') + def test_run_in_dir(self, mock): + instance = mock.return_value + instance.wait.return_value = 0 + instance.communicate.return_value = (self.output_ok, '') + ep = ExecProvider(self.input_ok, '/some/directory') + ep.run() + self.assertEqual(mock.call_args.kwargs['cwd'], '/some/directory') + if __name__ == '__main__': unittest.main() diff --git a/config/kube_config.py b/config/kube_config.py index a04a6e3e2..f37ed43ec 100644 --- a/config/kube_config.py +++ b/config/kube_config.py @@ -483,7 +483,8 @@ class KubeConfigLoader(object): if 'exec' not in self._user: return try: - status = ExecProvider(self._user['exec']).run() + base_path = self._get_base_path(self._cluster.path) + status = ExecProvider(self._user['exec'], base_path).run() if 'token' in status: self.token = "Bearer %s" % status['token'] elif 'clientCertificateData' in status: @@ -493,7 +494,6 @@ class KubeConfigLoader(object): logging.error('exec: missing clientKeyData field in ' 'plugin output') return None - base_path = self._get_base_path(self._cluster.path) self.cert_file = FileOrData( status, None, data_key_name='clientCertificateData', From 6efd33d5c16243929d32139d3b0d0bc34820ea7b Mon Sep 17 00:00:00 2001 From: April Schleck Date: Wed, 5 Jan 2022 17:56:07 -0800 Subject: [PATCH 2/2] Add a test to kube_config_test to check the cwd of the ExecProvider --- config/kube_config_test.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/config/kube_config_test.py b/config/kube_config_test.py index 6ac3db2da..02127d154 100644 --- a/config/kube_config_test.py +++ b/config/kube_config_test.py @@ -1441,6 +1441,20 @@ class TestKubeConfigLoader(BaseTestCase): active_context="exec_cred_user_certificate").load_and_set(actual) self.assertEqual(expected, actual) + @mock.patch('kubernetes.config.kube_config.ExecProvider.run', autospec=True) + def test_user_exec_cwd(self, mock): + capture = {} + def capture_cwd(exec_provider): + capture['cwd'] = exec_provider.cwd + mock.side_effect = capture_cwd + + expected = "/some/random/path" + KubeConfigLoader( + config_dict=self.TEST_KUBE_CONFIG, + active_context="exec_cred_user", + config_base_path=expected).load_and_set(FakeConfig()) + self.assertEqual(expected, capture['cwd']) + def test_user_cmd_path(self): A = namedtuple('A', ['token', 'expiry']) token = "dummy"