[AKS] Fix #33541: az aks install-cli: Fall back to the kubelogin version file when the GitHub API rate limit is hit - #34075
Tejas Mehta (mazbur) wants to merge 7 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
microsoft-github-policy-service
Bot
added
the
customer-reported
|
Thank you for your contribution Tejas Mehta (@mazbur)! We will review the pull request and get back to you soon. |
microsoft-github-policy-service
Bot
added
Auto-Assign
There was a problem hiding this comment.
🟡 Changes recommended
Moderate issues remain in fallback error handling and full version-response validation.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates az aks install-cli to fall back to the kubelogin version file when GitHub API rate limits occur.
Changes:
- Extracts version lookup and fallback handling.
- Validates and normalizes fallback versions.
- Adds coverage for fallback and failure scenarios.
File summaries
| File | Summary |
|---|---|
src/azure-cli/azure/cli/command_modules/acs/custom.py |
Implements kubelogin version lookup fallback and validation. |
src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py |
Tests fallback, validation, and error handling. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ex, fallback_url) | ||
| try: | ||
| latest_version = _urlopen_read(fallback_url).decode('UTF-8').strip() | ||
| except OSError as fallback_ex: |
There was a problem hiding this comment.
Adding UnicodeDecodeError
On ClientRequestError, I've left that uncaught deliberately. _urlopen_read only raises it for a local certificate store problem. Wrapping it would bury that guidance mid-string and attach a "retry later, or specify a version" recommendation that cannot help. Also, we only reach the fallback after the api call completed TLS successfully and answered 403 or 429, so the same process would have to fail TLS against github.com seconds later.
| '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): |
|
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
@microsoft-github-policy-service agree |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
🤖 PR Validation — ️✔️ All clear
Related command
az aks install-cliDescription
Resolving the latest kubelogin version depends on a single endpoint,
https://api.github.com/repos/Azure/kubelogin/releases/latest. GitHub rate limits unauthenticated requests to that API at 60 per hour per IP, which shared CI runners routinely exhaust, soaz aks install-clifails even though the release artifacts themselves are reachable.When the api reports a rate limit (err 403 or 429), the version is now read from the plain text file published as a release asset instead:
https://github.com/Azure/kubelogin/releases/latest/download/kubelogin-version.txt(added in Azure/kubelogin#749, requested in Azure/kubelogin discussion #748).The version lookup moves out of
k8s_install_kubelogininto_get_latest_kubelogin_versionso it can be covered without going through the download, unzip and PATH handling. Scope of the behaviour change:--gh-tokenwhen supplied.ClientRequestErrorreports both failures, so a rate limit can be told apart from any other error, and recommends--kubelogin-version.Unchanged: Azure China Cloud continues to use the
mirror.azure.cnendpoint with no GitHub fallback, an explicit--kubelogin-versionstill skips the lookup entirely, and the successful-api path behaves exactly as before.This relies on kubelogin publishing
kubelogin-version.txt.Testing Guide
Five unit tests were added, covering the end-to-end install via the fallback, that 429 also falls back while a 5xx and a connection error do not, the bare-version normalization, both endpoints failing, and a non-version response being rejected:
The full path was also exercised against the live endpoints, with only the api response simulated: a 403 is raised for
api.github.com.The version resolved from the version file produces a working kubelogin binary.
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.