From 4b74a6e69c68cb50c248c7d9bf5a4face33bb0bb Mon Sep 17 00:00:00 2001 From: Tejas Mehta Date: Sat, 12 Sep 2026 13:42:47 +0000 Subject: [PATCH 1/6] #33541 --- .../azure/cli/command_modules/acs/_consts.py | 8 +++ .../azure/cli/command_modules/acs/custom.py | 48 +++++++++++-- .../acs/tests/latest/test_custom.py | 68 +++++++++++++++++++ 3 files changed, 117 insertions(+), 7 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/_consts.py b/src/azure-cli/azure/cli/command_modules/acs/_consts.py index 227790750ee..c076d892249 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_consts.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_consts.py @@ -236,6 +236,14 @@ # consts for check-acr command CONST_CANIPULL_IMAGE = "mcr.microsoft.com/aks/canipull:v0.1.0" +# consts for install-cli command +CONST_KUBELOGIN_LATEST_RELEASE_URL = "https://api.github.com/repos/Azure/kubelogin/releases/latest" +# plain text file published as a kubelogin release asset, used as a fallback when the GitHub API is +# unavailable (e.g. when the unauthenticated rate limit is hit) +CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL = ( + "https://github.com/Azure/kubelogin/releases/latest/download/kubelogin-version.txt" +) + # consts for maintenance configuration schedule type CONST_DAILY_MAINTENANCE_SCHEDULE = "Daily" CONST_WEEKLY_MAINTENANCE_SCHEDULE = "Weekly" diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 02ec9df017d..06ab10e2030 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -58,6 +58,8 @@ CONST_INGRESS_APPGW_SUBNET_ID, CONST_INGRESS_APPGW_WATCH_NAMESPACE, CONST_KUBE_DASHBOARD_ADDON_NAME, + CONST_KUBELOGIN_LATEST_RELEASE_URL, + CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, CONST_MONITORING_ADDON_NAME, CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID, CONST_MONITORING_USING_AAD_MSI_AUTH, @@ -2573,6 +2575,44 @@ def k8s_install_kubectl(cmd, client_version='latest', install_location=None, sou install_dir, cli) +# get the latest version of kubelogin +def _get_latest_kubelogin_version(cloud_name, gh_token=None): + if cloud_name.lower() == 'azurechinacloud': + latest_release_url = 'https://mirror.azure.cn/kubernetes/kubelogin/latest' + logger.warning( + 'No version specified, will get the latest version of kubelogin from "%s"', latest_release_url) + latest_release = _urlopen_read(latest_release_url, gh_token=gh_token) + return json.loads(latest_release)['tag_name'].strip() + + latest_release_url = CONST_KUBELOGIN_LATEST_RELEASE_URL + logger.warning( + 'No version specified, will get the latest version of kubelogin from "%s"', latest_release_url) + try: + latest_release = _urlopen_read(latest_release_url, gh_token=gh_token) + return json.loads(latest_release)['tag_name'].strip() + # pylint: disable=broad-except + except Exception as ex: + # the GitHub API is rate limited to 60 requests per hour per IP address for unauthenticated + # requests, fall back to the version file published as a release asset, which is not rate limited + logger.warning( + 'Failed to get the latest version of kubelogin from "%s" (%s), falling back to "%s"', + latest_release_url, ex, CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL) + try: + latest_version = _urlopen_read(CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL).decode('UTF-8').strip() + # pylint: disable=broad-except + except Exception as fallback_ex: + raise CLIError( + 'Failed to get the latest version of kubelogin from "{}" ({}) and "{}" ({}). Please retry later or ' + 'specify the version with "--kubelogin-version".'.format( + latest_release_url, ex, CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, fallback_ex)) + if not re.match(r'^v?\d+\.\d+\.\d+', latest_version): + raise CLIError( + 'Unexpected version "{}" returned by "{}". Please retry later or specify the version with ' + '"--kubelogin-version".'.format(latest_version, CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL)) + # the version file holds the release tag (e.g. "v0.2.19"), normalize it in case the prefix is missing + return latest_version if latest_version.startswith('v') else 'v' + latest_version + + # install kubelogin def k8s_install_kubelogin(cmd, client_version='latest', install_location=None, source_url=None, arch=None, gh_token=None): """ @@ -2587,13 +2627,7 @@ def k8s_install_kubelogin(cmd, client_version='latest', install_location=None, s source_url = 'https://mirror.azure.cn/kubernetes/kubelogin' if client_version == 'latest': - latest_release_url = 'https://api.github.com/repos/Azure/kubelogin/releases/latest' - if cloud_name.lower() == 'azurechinacloud': - latest_release_url = 'https://mirror.azure.cn/kubernetes/kubelogin/latest' - logger.warning( - 'No version specified, will get the latest version of kubelogin from "%s"', latest_release_url) - latest_release = _urlopen_read(latest_release_url, gh_token=gh_token) - client_version = json.loads(latest_release)['tag_name'].strip() + client_version = _get_latest_kubelogin_version(cloud_name, gh_token=gh_token) else: client_version = "v%s" % client_version diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py index 2e12429ac29..f372f1aa2d6 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py @@ -8,6 +8,7 @@ import tempfile import unittest from unittest import mock +from urllib.error import HTTPError import datetime from dateutil.parser import parse @@ -16,6 +17,8 @@ CONST_AZURE_POLICY_ADDON_NAME, CONST_HTTP_APPLICATION_ROUTING_ADDON_NAME, CONST_KUBE_DASHBOARD_ADDON_NAME, + CONST_KUBELOGIN_LATEST_RELEASE_URL, + CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, CONST_MONITORING_ADDON_NAME, CONST_MONITORING_USING_AAD_MSI_AUTH, ) @@ -25,6 +28,7 @@ ) from azure.cli.command_modules.acs.custom import ( _get_command_context, + _get_latest_kubelogin_version, _update_addons, aks_agentpool_auto_scale_add, aks_agentpool_auto_scale_delete, @@ -848,6 +852,70 @@ def test_k8s_install_kubelogin_with_gh_token(self, logger_mock, mock_url_retriev finally: shutil.rmtree(temp_dir) + @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') + @mock.patch('azure.cli.command_modules.acs.custom._urlretrieve') + @mock.patch('azure.cli.command_modules.acs.custom.logger') + def test_k8s_install_kubelogin_latest_version_fallback(self, logger_mock, mock_url_retrieve, mock_urlopen_read): + """Test that the version file is used when the GitHub API fails, e.g. due to rate limiting.""" + rate_limited = HTTPError(CONST_KUBELOGIN_LATEST_RELEASE_URL, 403, 'rate limit exceeded', None, None) + mock_urlopen_read.side_effect = [rate_limited, b'v0.0.30'] + mock_url_retrieve.side_effect = create_kubelogin_zip + + try: + temp_dir = tempfile.mkdtemp() + test_location = os.path.join(temp_dir, 'foo', 'kubelogin') + + k8s_install_kubelogin( + mock.MagicMock(), client_version='latest', install_location=test_location, arch="amd64") + + # the version file is downloaded without the GitHub API + self.assertEqual(mock_urlopen_read.call_count, 2) + self.assertEqual( + mock_urlopen_read.call_args_list[1][0][0], CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL) + # the version from the version file is used to build the download url + mock_url_retrieve.assert_called_with( + MockUrlretrieveUrlValidator('https://github.com/Azure/kubelogin/releases/download', 'v0.0.30'), + mock.ANY) + self.assertTrue(os.path.exists(test_location)) + finally: + shutil.rmtree(temp_dir) + + @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') + @mock.patch('azure.cli.command_modules.acs.custom.logger') + def test_get_latest_kubelogin_version_all_sources_fail(self, logger_mock, mock_urlopen_read): + """Test that a clear error is raised when both the GitHub API and the version file are unavailable.""" + mock_urlopen_read.side_effect = [ + HTTPError(CONST_KUBELOGIN_LATEST_RELEASE_URL, 403, 'rate limit exceeded', None, None), + HTTPError(CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, 500, 'internal server error', None, None), + ] + + with self.assertRaises(CLIError): + _get_latest_kubelogin_version('azurecloud') + self.assertEqual(mock_urlopen_read.call_count, 2) + + @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') + @mock.patch('azure.cli.command_modules.acs.custom.logger') + def test_get_latest_kubelogin_version_unexpected_fallback_content(self, logger_mock, mock_urlopen_read): + """Test that content which is not a version (e.g. an html error page) is rejected.""" + mock_urlopen_read.side_effect = [ + HTTPError(CONST_KUBELOGIN_LATEST_RELEASE_URL, 403, 'rate limit exceeded', None, None), + b'not found', + ] + + with self.assertRaises(CLIError): + _get_latest_kubelogin_version('azurecloud') + + @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') + @mock.patch('azure.cli.command_modules.acs.custom.logger') + def test_get_latest_kubelogin_version_china_cloud(self, logger_mock, mock_urlopen_read): + """Test that the china cloud mirror is used as is, without the GitHub fallback.""" + mock_urlopen_read.return_value = b'{"tag_name": "v0.0.30"}' + + self.assertEqual(_get_latest_kubelogin_version('AzureChinaCloud'), 'v0.0.30') + mock_urlopen_read.assert_called_once() + self.assertEqual( + mock_urlopen_read.call_args[0][0], 'https://mirror.azure.cn/kubernetes/kubelogin/latest') + @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_rg_location', return_value='eastus') @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resource_groups_client', autospec=True) @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resources_client', autospec=True) From 2cb53bfbf5cfaac28c383c456ada85887a1b6b43 Mon Sep 17 00:00:00 2001 From: Tejas Mehta Date: Sun, 13 Sep 2026 12:29:29 +0000 Subject: [PATCH 2/6] replaced CLIError with azureclierror --- .../azure/cli/command_modules/acs/custom.py | 15 ++++++++------- .../acs/tests/latest/test_custom.py | 5 +++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 06ab10e2030..4a7ab3fbca4 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -2601,14 +2601,15 @@ def _get_latest_kubelogin_version(cloud_name, gh_token=None): latest_version = _urlopen_read(CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL).decode('UTF-8').strip() # pylint: disable=broad-except except Exception as fallback_ex: - raise CLIError( - 'Failed to get the latest version of kubelogin from "{}" ({}) and "{}" ({}). Please retry later or ' - 'specify the version with "--kubelogin-version".'.format( - latest_release_url, ex, CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, fallback_ex)) + raise ClientRequestError( + 'Failed to get the latest version of kubelogin from "{}" ({}) and "{}" ({}).'.format( + latest_release_url, ex, CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, fallback_ex), + recommendation='Please retry later, or specify a version with --kubelogin-version.') if not re.match(r'^v?\d+\.\d+\.\d+', latest_version): - raise CLIError( - 'Unexpected version "{}" returned by "{}". Please retry later or specify the version with ' - '"--kubelogin-version".'.format(latest_version, CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL)) + raise ClientRequestError( + 'Unexpected version "{}" returned by "{}".'.format( + latest_version, CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL), + recommendation='Please retry later, or specify a version with --kubelogin-version.') # the version file holds the release tag (e.g. "v0.2.19"), normalize it in case the prefix is missing return latest_version if latest_version.startswith('v') else 'v' + latest_version diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py index f372f1aa2d6..0ebd8a354f1 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py @@ -58,6 +58,7 @@ create_kubelogin_zip, get_test_data_file_path, ) +from azure.cli.core.azclierror import ClientRequestError from azure.cli.core.util import CLIError from azure.cli.core.profiles import ResourceType from azure.core.exceptions import HttpResponseError @@ -889,7 +890,7 @@ def test_get_latest_kubelogin_version_all_sources_fail(self, logger_mock, mock_u HTTPError(CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, 500, 'internal server error', None, None), ] - with self.assertRaises(CLIError): + with self.assertRaises(ClientRequestError): _get_latest_kubelogin_version('azurecloud') self.assertEqual(mock_urlopen_read.call_count, 2) @@ -902,7 +903,7 @@ def test_get_latest_kubelogin_version_unexpected_fallback_content(self, logger_m b'not found', ] - with self.assertRaises(CLIError): + with self.assertRaises(ClientRequestError): _get_latest_kubelogin_version('azurecloud') @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') From a64110d4a2ba11719786f37c17ad037686de7a7c Mon Sep 17 00:00:00 2001 From: Tejas Mehta Date: Sun, 13 Sep 2026 18:22:58 +0000 Subject: [PATCH 3/6] refactor --- .../azure/cli/command_modules/acs/_consts.py | 8 --- .../azure/cli/command_modules/acs/custom.py | 29 ++++---- .../acs/tests/latest/test_custom.py | 68 ++++++++++++++----- 3 files changed, 67 insertions(+), 38 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/_consts.py b/src/azure-cli/azure/cli/command_modules/acs/_consts.py index c076d892249..227790750ee 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_consts.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_consts.py @@ -236,14 +236,6 @@ # consts for check-acr command CONST_CANIPULL_IMAGE = "mcr.microsoft.com/aks/canipull:v0.1.0" -# consts for install-cli command -CONST_KUBELOGIN_LATEST_RELEASE_URL = "https://api.github.com/repos/Azure/kubelogin/releases/latest" -# plain text file published as a kubelogin release asset, used as a fallback when the GitHub API is -# unavailable (e.g. when the unauthenticated rate limit is hit) -CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL = ( - "https://github.com/Azure/kubelogin/releases/latest/download/kubelogin-version.txt" -) - # consts for maintenance configuration schedule type CONST_DAILY_MAINTENANCE_SCHEDULE = "Daily" CONST_WEEKLY_MAINTENANCE_SCHEDULE = "Weekly" diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 4a7ab3fbca4..3c5f962a14c 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -58,8 +58,6 @@ CONST_INGRESS_APPGW_SUBNET_ID, CONST_INGRESS_APPGW_WATCH_NAMESPACE, CONST_KUBE_DASHBOARD_ADDON_NAME, - CONST_KUBELOGIN_LATEST_RELEASE_URL, - CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, CONST_MONITORING_ADDON_NAME, CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID, CONST_MONITORING_USING_AAD_MSI_AUTH, @@ -2584,31 +2582,34 @@ def _get_latest_kubelogin_version(cloud_name, gh_token=None): latest_release = _urlopen_read(latest_release_url, gh_token=gh_token) return json.loads(latest_release)['tag_name'].strip() - latest_release_url = CONST_KUBELOGIN_LATEST_RELEASE_URL + latest_release_url = 'https://api.github.com/repos/Azure/kubelogin/releases/latest' + # plain text file published as a release asset, it is not subject to the GitHub API rate limit + # of 60 requests per hour per IP address for unauthenticated requests + fallback_url = 'https://github.com/Azure/kubelogin/releases/latest/download/kubelogin-version.txt' logger.warning( 'No version specified, will get the latest version of kubelogin from "%s"', latest_release_url) + # OSError covers URLError/HTTPError (e.g. a 403 when rate limited), ValueError and KeyError cover a + # response that is not the expected json. A ClientRequestError raised by _urlopen_read is not caught, + # it reports a local issue (e.g. an unusable cert store) that the fallback url would hit as well. try: latest_release = _urlopen_read(latest_release_url, gh_token=gh_token) return json.loads(latest_release)['tag_name'].strip() - # pylint: disable=broad-except - except Exception as ex: - # the GitHub API is rate limited to 60 requests per hour per IP address for unauthenticated - # requests, fall back to the version file published as a release asset, which is not rate limited + except (OSError, ValueError, KeyError) as ex: logger.warning( 'Failed to get the latest version of kubelogin from "%s" (%s), falling back to "%s"', - latest_release_url, ex, CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL) + latest_release_url, ex, fallback_url) try: - latest_version = _urlopen_read(CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL).decode('UTF-8').strip() - # pylint: disable=broad-except - except Exception as fallback_ex: + # the token is deliberately not sent here, the release asset is served by a redirect to a + # storage endpoint which rejects requests carrying an unexpected Authorization header + latest_version = _urlopen_read(fallback_url).decode('UTF-8').strip() + except OSError as fallback_ex: raise ClientRequestError( 'Failed to get the latest version of kubelogin from "{}" ({}) and "{}" ({}).'.format( - latest_release_url, ex, CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, fallback_ex), + latest_release_url, ex, fallback_url, fallback_ex), recommendation='Please retry later, or specify a version with --kubelogin-version.') if not re.match(r'^v?\d+\.\d+\.\d+', latest_version): raise ClientRequestError( - 'Unexpected version "{}" returned by "{}".'.format( - latest_version, CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL), + 'Unexpected version "{}" returned by "{}".'.format(latest_version, fallback_url), recommendation='Please retry later, or specify a version with --kubelogin-version.') # the version file holds the release tag (e.g. "v0.2.19"), normalize it in case the prefix is missing return latest_version if latest_version.startswith('v') else 'v' + latest_version diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py index 0ebd8a354f1..40a990c07a7 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py @@ -17,8 +17,6 @@ CONST_AZURE_POLICY_ADDON_NAME, CONST_HTTP_APPLICATION_ROUTING_ADDON_NAME, CONST_KUBE_DASHBOARD_ADDON_NAME, - CONST_KUBELOGIN_LATEST_RELEASE_URL, - CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, CONST_MONITORING_ADDON_NAME, CONST_MONITORING_USING_AAD_MSI_AUTH, ) @@ -72,6 +70,10 @@ ) +GITHUB_API_URL = 'https://api.github.com/repos/Azure/kubelogin/releases/latest' +VERSION_FILE_URL = 'https://github.com/Azure/kubelogin/releases/latest/download/kubelogin-version.txt' + + class AcsCustomCommandTest(unittest.TestCase): def setUp(self): self.cli = MockCLI() @@ -857,9 +859,8 @@ def test_k8s_install_kubelogin_with_gh_token(self, logger_mock, mock_url_retriev @mock.patch('azure.cli.command_modules.acs.custom._urlretrieve') @mock.patch('azure.cli.command_modules.acs.custom.logger') def test_k8s_install_kubelogin_latest_version_fallback(self, logger_mock, mock_url_retrieve, mock_urlopen_read): - """Test that the version file is used when the GitHub API fails, e.g. due to rate limiting.""" - rate_limited = HTTPError(CONST_KUBELOGIN_LATEST_RELEASE_URL, 403, 'rate limit exceeded', None, None) - mock_urlopen_read.side_effect = [rate_limited, b'v0.0.30'] + """Test that the version file is used to install kubelogin when the GitHub API is rate limited.""" + mock_urlopen_read.side_effect = [HTTPError(GITHUB_API_URL, 403, 'rate limit exceeded', None, None), b'v0.0.30'] mock_url_retrieve.side_effect = create_kubelogin_zip try: @@ -867,12 +868,13 @@ def test_k8s_install_kubelogin_latest_version_fallback(self, logger_mock, mock_u test_location = os.path.join(temp_dir, 'foo', 'kubelogin') k8s_install_kubelogin( - mock.MagicMock(), client_version='latest', install_location=test_location, arch="amd64") + mock.MagicMock(), client_version='latest', install_location=test_location, + arch="amd64", gh_token='ghp_test_token_123') - # the version file is downloaded without the GitHub API - self.assertEqual(mock_urlopen_read.call_count, 2) - self.assertEqual( - mock_urlopen_read.call_args_list[1][0][0], CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL) + fallback_call = mock_urlopen_read.call_args_list[1] + self.assertEqual(fallback_call[0][0], VERSION_FILE_URL) + # the release asset is served by a redirect that rejects an unexpected Authorization header + self.assertIsNone(fallback_call.kwargs.get('gh_token')) # the version from the version file is used to build the download url mock_url_retrieve.assert_called_with( MockUrlretrieveUrlValidator('https://github.com/Azure/kubelogin/releases/download', 'v0.0.30'), @@ -881,35 +883,69 @@ def test_k8s_install_kubelogin_latest_version_fallback(self, logger_mock, mock_u finally: shutil.rmtree(temp_dir) + @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') + @mock.patch('azure.cli.command_modules.acs.custom.logger') + def test_get_latest_kubelogin_version_unusable_api_response(self, logger_mock, mock_urlopen_read): + """Test that a response which is not the expected json also triggers the fallback.""" + for api_response in (b'rate limit', b'{"message": "API rate limit exceeded"}'): + mock_urlopen_read.reset_mock() + mock_urlopen_read.side_effect = [api_response, b'v0.0.30'] + + self.assertEqual(_get_latest_kubelogin_version('azurecloud'), 'v0.0.30') + self.assertEqual(mock_urlopen_read.call_count, 2) + + @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') + @mock.patch('azure.cli.command_modules.acs.custom.logger') + def test_get_latest_kubelogin_version_fallback_without_tag_prefix(self, logger_mock, mock_urlopen_read): + """Test that a bare version is normalized to the release tag used to build the download url.""" + mock_urlopen_read.side_effect = [HTTPError(GITHUB_API_URL, 403, 'rate limited', None, None), b'0.0.30\n'] + + self.assertEqual(_get_latest_kubelogin_version('azurecloud'), 'v0.0.30') + @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') @mock.patch('azure.cli.command_modules.acs.custom.logger') def test_get_latest_kubelogin_version_all_sources_fail(self, logger_mock, mock_urlopen_read): """Test that a clear error is raised when both the GitHub API and the version file are unavailable.""" mock_urlopen_read.side_effect = [ - HTTPError(CONST_KUBELOGIN_LATEST_RELEASE_URL, 403, 'rate limit exceeded', None, None), - HTTPError(CONST_KUBELOGIN_LATEST_VERSION_FALLBACK_URL, 500, 'internal server error', None, None), + HTTPError(GITHUB_API_URL, 403, 'rate limit exceeded', None, None), + HTTPError(VERSION_FILE_URL, 500, 'internal server error', None, None), ] - with self.assertRaises(ClientRequestError): + with self.assertRaises(ClientRequestError) as cm: _get_latest_kubelogin_version('azurecloud') - self.assertEqual(mock_urlopen_read.call_count, 2) + # both failures are reported so a rate limit can be told apart from an outage + self.assertIn('403', str(cm.exception)) + self.assertIn('500', str(cm.exception)) @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') @mock.patch('azure.cli.command_modules.acs.custom.logger') def test_get_latest_kubelogin_version_unexpected_fallback_content(self, logger_mock, mock_urlopen_read): """Test that content which is not a version (e.g. an html error page) is rejected.""" mock_urlopen_read.side_effect = [ - HTTPError(CONST_KUBELOGIN_LATEST_RELEASE_URL, 403, 'rate limit exceeded', None, None), + HTTPError(GITHUB_API_URL, 403, 'rate limit exceeded', None, None), b'not found', ] with self.assertRaises(ClientRequestError): _get_latest_kubelogin_version('azurecloud') + @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') + @mock.patch('azure.cli.command_modules.acs.custom.logger') + def test_get_latest_kubelogin_version_local_error_not_retried(self, logger_mock, mock_urlopen_read): + """Test that a local error (e.g. an unusable cert store) is surfaced instead of hitting the fallback.""" + ssl_error = ClientRequestError('SSL certificate verification failed.') + mock_urlopen_read.side_effect = ssl_error + + with self.assertRaises(ClientRequestError) as cm: + _get_latest_kubelogin_version('azurecloud') + # the original actionable error is preserved and the fallback url is not requested + self.assertIs(cm.exception, ssl_error) + mock_urlopen_read.assert_called_once() + @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') @mock.patch('azure.cli.command_modules.acs.custom.logger') def test_get_latest_kubelogin_version_china_cloud(self, logger_mock, mock_urlopen_read): - """Test that the china cloud mirror is used as is, without the GitHub fallback.""" + """Test that the china cloud mirror is used, the GitHub fallback is not reachable from there.""" mock_urlopen_read.return_value = b'{"tag_name": "v0.0.30"}' self.assertEqual(_get_latest_kubelogin_version('AzureChinaCloud'), 'v0.0.30') From 4f57b35fde1a2cdaebe6cfceb7c5114981cef55d Mon Sep 17 00:00:00 2001 From: Tejas Mehta Date: Sun, 13 Sep 2026 19:31:10 +0000 Subject: [PATCH 4/6] refactor --- .../azure/cli/command_modules/acs/custom.py | 9 +--- .../acs/tests/latest/test_custom.py | 42 ++++++++++--------- 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 3c5f962a14c..a01065ea7ff 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -2583,24 +2583,18 @@ def _get_latest_kubelogin_version(cloud_name, gh_token=None): return json.loads(latest_release)['tag_name'].strip() latest_release_url = 'https://api.github.com/repos/Azure/kubelogin/releases/latest' - # plain text file published as a release asset, it is not subject to the GitHub API rate limit - # of 60 requests per hour per IP address for unauthenticated requests fallback_url = 'https://github.com/Azure/kubelogin/releases/latest/download/kubelogin-version.txt' logger.warning( 'No version specified, will get the latest version of kubelogin from "%s"', latest_release_url) - # OSError covers URLError/HTTPError (e.g. a 403 when rate limited), ValueError and KeyError cover a - # response that is not the expected json. A ClientRequestError raised by _urlopen_read is not caught, - # it reports a local issue (e.g. an unusable cert store) that the fallback url would hit as well. try: latest_release = _urlopen_read(latest_release_url, gh_token=gh_token) return json.loads(latest_release)['tag_name'].strip() except (OSError, ValueError, KeyError) as ex: + # fall back to the version file, it is not subject to the GitHub API rate limit logger.warning( 'Failed to get the latest version of kubelogin from "%s" (%s), falling back to "%s"', latest_release_url, ex, fallback_url) try: - # the token is deliberately not sent here, the release asset is served by a redirect to a - # storage endpoint which rejects requests carrying an unexpected Authorization header latest_version = _urlopen_read(fallback_url).decode('UTF-8').strip() except OSError as fallback_ex: raise ClientRequestError( @@ -2611,7 +2605,6 @@ def _get_latest_kubelogin_version(cloud_name, gh_token=None): raise ClientRequestError( 'Unexpected version "{}" returned by "{}".'.format(latest_version, fallback_url), recommendation='Please retry later, or specify a version with --kubelogin-version.') - # the version file holds the release tag (e.g. "v0.2.19"), normalize it in case the prefix is missing return latest_version if latest_version.startswith('v') else 'v' + latest_version diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py index 40a990c07a7..7774e025ed6 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py @@ -70,10 +70,6 @@ ) -GITHUB_API_URL = 'https://api.github.com/repos/Azure/kubelogin/releases/latest' -VERSION_FILE_URL = 'https://github.com/Azure/kubelogin/releases/latest/download/kubelogin-version.txt' - - class AcsCustomCommandTest(unittest.TestCase): def setUp(self): self.cli = MockCLI() @@ -860,7 +856,10 @@ def test_k8s_install_kubelogin_with_gh_token(self, logger_mock, mock_url_retriev @mock.patch('azure.cli.command_modules.acs.custom.logger') def test_k8s_install_kubelogin_latest_version_fallback(self, logger_mock, mock_url_retrieve, mock_urlopen_read): """Test that the version file is used to install kubelogin when the GitHub API is rate limited.""" - mock_urlopen_read.side_effect = [HTTPError(GITHUB_API_URL, 403, 'rate limit exceeded', None, None), b'v0.0.30'] + mock_urlopen_read.side_effect = [ + HTTPError('https://api.github.com/repos/Azure/kubelogin/releases/latest', 403, 'rate limited', None, None), + b'v0.0.30', + ] mock_url_retrieve.side_effect = create_kubelogin_zip try: @@ -872,13 +871,15 @@ def test_k8s_install_kubelogin_latest_version_fallback(self, logger_mock, mock_u arch="amd64", gh_token='ghp_test_token_123') fallback_call = mock_urlopen_read.call_args_list[1] - self.assertEqual(fallback_call[0][0], VERSION_FILE_URL) - # the release asset is served by a redirect that rejects an unexpected Authorization header + self.assertEqual( + fallback_call[0][0], + 'https://github.com/Azure/kubelogin/releases/latest/download/kubelogin-version.txt') self.assertIsNone(fallback_call.kwargs.get('gh_token')) - # the version from the version file is used to build the download url mock_url_retrieve.assert_called_with( MockUrlretrieveUrlValidator('https://github.com/Azure/kubelogin/releases/download', 'v0.0.30'), mock.ANY) + self.assertTrue( + any('falling back' in str(call) for call in logger_mock.warning.call_args_list)) self.assertTrue(os.path.exists(test_location)) finally: shutil.rmtree(temp_dir) @@ -888,32 +889,36 @@ def test_k8s_install_kubelogin_latest_version_fallback(self, logger_mock, mock_u def test_get_latest_kubelogin_version_unusable_api_response(self, logger_mock, mock_urlopen_read): """Test that a response which is not the expected json also triggers the fallback.""" for api_response in (b'rate limit', b'{"message": "API rate limit exceeded"}'): - mock_urlopen_read.reset_mock() - mock_urlopen_read.side_effect = [api_response, b'v0.0.30'] + with self.subTest(api_response=api_response): + mock_urlopen_read.reset_mock() + mock_urlopen_read.side_effect = [api_response, b'v0.0.30'] - self.assertEqual(_get_latest_kubelogin_version('azurecloud'), 'v0.0.30') - self.assertEqual(mock_urlopen_read.call_count, 2) + self.assertEqual(_get_latest_kubelogin_version('azurecloud'), 'v0.0.30') + self.assertEqual(mock_urlopen_read.call_count, 2) @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') @mock.patch('azure.cli.command_modules.acs.custom.logger') def test_get_latest_kubelogin_version_fallback_without_tag_prefix(self, logger_mock, mock_urlopen_read): """Test that a bare version is normalized to the release tag used to build the download url.""" - mock_urlopen_read.side_effect = [HTTPError(GITHUB_API_URL, 403, 'rate limited', None, None), b'0.0.30\n'] + mock_urlopen_read.side_effect = [ + HTTPError('https://api.github.com/repos/Azure/kubelogin/releases/latest', 403, 'rate limited', None, None), + b'0.0.30\n', + ] self.assertEqual(_get_latest_kubelogin_version('azurecloud'), 'v0.0.30') @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') @mock.patch('azure.cli.command_modules.acs.custom.logger') def test_get_latest_kubelogin_version_all_sources_fail(self, logger_mock, mock_urlopen_read): - """Test that a clear error is raised when both the GitHub API and the version file are unavailable.""" + """Test that both failures are reported when the GitHub API and the version file are unavailable.""" mock_urlopen_read.side_effect = [ - HTTPError(GITHUB_API_URL, 403, 'rate limit exceeded', None, None), - HTTPError(VERSION_FILE_URL, 500, 'internal server error', None, None), + HTTPError('https://api.github.com/repos/Azure/kubelogin/releases/latest', 403, 'rate limited', None, None), + HTTPError('https://github.com/Azure/kubelogin/releases/latest/download/kubelogin-version.txt', + 500, 'internal server error', None, None), ] with self.assertRaises(ClientRequestError) as cm: _get_latest_kubelogin_version('azurecloud') - # both failures are reported so a rate limit can be told apart from an outage self.assertIn('403', str(cm.exception)) self.assertIn('500', str(cm.exception)) @@ -922,7 +927,7 @@ def test_get_latest_kubelogin_version_all_sources_fail(self, logger_mock, mock_u def test_get_latest_kubelogin_version_unexpected_fallback_content(self, logger_mock, mock_urlopen_read): """Test that content which is not a version (e.g. an html error page) is rejected.""" mock_urlopen_read.side_effect = [ - HTTPError(GITHUB_API_URL, 403, 'rate limit exceeded', None, None), + HTTPError('https://api.github.com/repos/Azure/kubelogin/releases/latest', 403, 'rate limited', None, None), b'not found', ] @@ -938,7 +943,6 @@ def test_get_latest_kubelogin_version_local_error_not_retried(self, logger_mock, with self.assertRaises(ClientRequestError) as cm: _get_latest_kubelogin_version('azurecloud') - # the original actionable error is preserved and the fallback url is not requested self.assertIs(cm.exception, ssl_error) mock_urlopen_read.assert_called_once() From e6040f5929f036df0246fc1c377b747e837c1557 Mon Sep 17 00:00:00 2001 From: Tejas Mehta Date: Sun, 13 Sep 2026 20:31:16 +0000 Subject: [PATCH 5/6] refactor --- .../azure/cli/command_modules/acs/custom.py | 10 ++-- .../acs/tests/latest/test_custom.py | 55 +++++++------------ 2 files changed, 27 insertions(+), 38 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index a01065ea7ff..db2f7c9498f 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -2589,11 +2589,13 @@ def _get_latest_kubelogin_version(cloud_name, gh_token=None): try: latest_release = _urlopen_read(latest_release_url, gh_token=gh_token) return json.loads(latest_release)['tag_name'].strip() - except (OSError, ValueError, KeyError) as ex: - # fall back to the version file, it is not subject to the GitHub API rate limit + except URLError as ex: + # the GitHub api answers with 403 or 429 when the rate limit is exceeded + if getattr(ex, 'code', None) not in (403, 429): + raise logger.warning( - 'Failed to get the latest version of kubelogin from "%s" (%s), falling back to "%s"', - latest_release_url, ex, fallback_url) + 'The GitHub api rate limit was exceeded (%s), getting the latest version of kubelogin from "%s"', + ex, fallback_url) try: latest_version = _urlopen_read(fallback_url).decode('UTF-8').strip() except OSError as fallback_ex: diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py index 7774e025ed6..4931a296fd0 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py @@ -8,7 +8,7 @@ import tempfile import unittest from unittest import mock -from urllib.error import HTTPError +from urllib.error import HTTPError, URLError import datetime from dateutil.parser import parse @@ -56,7 +56,6 @@ create_kubelogin_zip, get_test_data_file_path, ) -from azure.cli.core.azclierror import ClientRequestError from azure.cli.core.util import CLIError from azure.cli.core.profiles import ResourceType from azure.core.exceptions import HttpResponseError @@ -879,22 +878,33 @@ def test_k8s_install_kubelogin_latest_version_fallback(self, logger_mock, mock_u MockUrlretrieveUrlValidator('https://github.com/Azure/kubelogin/releases/download', 'v0.0.30'), mock.ANY) self.assertTrue( - any('falling back' in str(call) for call in logger_mock.warning.call_args_list)) - self.assertTrue(os.path.exists(test_location)) + any('rate limit was exceeded' in str(call) for call in logger_mock.warning.call_args_list)) finally: shutil.rmtree(temp_dir) @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') @mock.patch('azure.cli.command_modules.acs.custom.logger') - def test_get_latest_kubelogin_version_unusable_api_response(self, logger_mock, mock_urlopen_read): - """Test that a response which is not the expected json also triggers the fallback.""" - for api_response in (b'rate limit', b'{"message": "API rate limit exceeded"}'): - with self.subTest(api_response=api_response): + def test_get_latest_kubelogin_version_fallback_only_on_rate_limit(self, logger_mock, mock_urlopen_read): + """Test that the version file is only used for a rate limit, other failures are surfaced as they are.""" + api_url = 'https://api.github.com/repos/Azure/kubelogin/releases/latest' + cases = [ + (HTTPError(api_url, 429, 'too many requests', None, None), True), + (HTTPError(api_url, 500, 'internal server error', None, None), False), + (URLError('[Errno -2] Name or service not known'), False), + ] + for error, expect_fallback in cases: + with self.subTest(error=error): mock_urlopen_read.reset_mock() - mock_urlopen_read.side_effect = [api_response, b'v0.0.30'] + mock_urlopen_read.side_effect = [error, b'v0.0.30'] - self.assertEqual(_get_latest_kubelogin_version('azurecloud'), 'v0.0.30') - self.assertEqual(mock_urlopen_read.call_count, 2) + if expect_fallback: + self.assertEqual(_get_latest_kubelogin_version('azurecloud'), 'v0.0.30') + self.assertEqual(mock_urlopen_read.call_count, 2) + else: + with self.assertRaises(type(error)) as cm: + _get_latest_kubelogin_version('azurecloud') + self.assertIs(cm.exception, error) + mock_urlopen_read.assert_called_once() @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') @mock.patch('azure.cli.command_modules.acs.custom.logger') @@ -934,29 +944,6 @@ def test_get_latest_kubelogin_version_unexpected_fallback_content(self, logger_m with self.assertRaises(ClientRequestError): _get_latest_kubelogin_version('azurecloud') - @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') - @mock.patch('azure.cli.command_modules.acs.custom.logger') - def test_get_latest_kubelogin_version_local_error_not_retried(self, logger_mock, mock_urlopen_read): - """Test that a local error (e.g. an unusable cert store) is surfaced instead of hitting the fallback.""" - ssl_error = ClientRequestError('SSL certificate verification failed.') - mock_urlopen_read.side_effect = ssl_error - - with self.assertRaises(ClientRequestError) as cm: - _get_latest_kubelogin_version('azurecloud') - self.assertIs(cm.exception, ssl_error) - mock_urlopen_read.assert_called_once() - - @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') - @mock.patch('azure.cli.command_modules.acs.custom.logger') - def test_get_latest_kubelogin_version_china_cloud(self, logger_mock, mock_urlopen_read): - """Test that the china cloud mirror is used, the GitHub fallback is not reachable from there.""" - mock_urlopen_read.return_value = b'{"tag_name": "v0.0.30"}' - - self.assertEqual(_get_latest_kubelogin_version('AzureChinaCloud'), 'v0.0.30') - mock_urlopen_read.assert_called_once() - self.assertEqual( - mock_urlopen_read.call_args[0][0], 'https://mirror.azure.cn/kubernetes/kubelogin/latest') - @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_rg_location', return_value='eastus') @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resource_groups_client', autospec=True) @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resources_client', autospec=True) From 1511d133d350b3a563aed4b0f64f92ebf4958a24 Mon Sep 17 00:00:00 2001 From: Tejas Mehta Date: Tue, 15 Sep 2026 09:05:38 +0000 Subject: [PATCH 6/6] fixed a bug and updated test --- .../azure/cli/command_modules/acs/custom.py | 6 +++--- .../acs/tests/latest/test_custom.py | 20 +++++++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index db2f7c9498f..06ca9920406 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -2597,15 +2597,15 @@ def _get_latest_kubelogin_version(cloud_name, gh_token=None): 'The GitHub api rate limit was exceeded (%s), getting the latest version of kubelogin from "%s"', ex, fallback_url) try: - latest_version = _urlopen_read(fallback_url).decode('UTF-8').strip() + latest_version = _urlopen_read(fallback_url).decode('UTF-8', errors='replace').strip() except OSError as fallback_ex: raise ClientRequestError( 'Failed to get the latest version of kubelogin from "{}" ({}) and "{}" ({}).'.format( latest_release_url, ex, fallback_url, fallback_ex), recommendation='Please retry later, or specify a version with --kubelogin-version.') - if not re.match(r'^v?\d+\.\d+\.\d+', latest_version): + if not re.fullmatch(r'v?\d+\.\d+\.\d+', latest_version): raise ClientRequestError( - 'Unexpected version "{}" returned by "{}".'.format(latest_version, fallback_url), + 'Unexpected version "{}" returned by "{}".'.format(latest_version[:50], fallback_url), recommendation='Please retry later, or specify a version with --kubelogin-version.') return latest_version if latest_version.startswith('v') else 'v' + latest_version diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py index 4931a296fd0..50cb48971f8 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py @@ -935,14 +935,18 @@ def test_get_latest_kubelogin_version_all_sources_fail(self, logger_mock, mock_u @mock.patch('azure.cli.command_modules.acs.custom._urlopen_read') @mock.patch('azure.cli.command_modules.acs.custom.logger') def test_get_latest_kubelogin_version_unexpected_fallback_content(self, logger_mock, mock_urlopen_read): - """Test that content which is not a version (e.g. an html error page) is rejected.""" - mock_urlopen_read.side_effect = [ - HTTPError('https://api.github.com/repos/Azure/kubelogin/releases/latest', 403, 'rate limited', None, None), - b'not found', - ] - - with self.assertRaises(ClientRequestError): - _get_latest_kubelogin_version('azurecloud') + """Test that content which is not exactly a version is rejected, not used to build the download url.""" + for content in (b'not found', b'v0.0.30/../../evil', b'\xff\xfe\x00binary'): + with self.subTest(content=content): + mock_urlopen_read.reset_mock() + mock_urlopen_read.side_effect = [ + HTTPError('https://api.github.com/repos/Azure/kubelogin/releases/latest', + 403, 'rate limited', None, None), + content, + ] + + with self.assertRaises(ClientRequestError): + _get_latest_kubelogin_version('azurecloud') @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_rg_location', return_value='eastus') @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resource_groups_client', autospec=True)