[AKS] Fix #34091: az aks get-credentials: Fix kubelogin conversion for explicit kubeconfig paths - #34099
Open
Aditya Pujara (a0x1ab) wants to merge 1 commit into
Conversation
…sion for explicit kubeconfig paths AI-generated by X Engineering Agent via Azure AI Foundry. Foundry-Job: 5cea4d61c0405a0d3cad2061ebf19ec75d5984a5d77936de831da998855811b1
x-engineering-agent
Bot
added
the
Request X Engineering Agent
microsoft-github-policy-service
Bot
requested review from
FumingZhang,
elvazhu521,
Julie Zhu (yanzhudd) and
Yong Zhang (yonzhan)
September 18, 2026 15:16
microsoft-github-policy-service
Bot
added
Auto-Assign
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate issues remain in kubelogin execution and explicit-path recovery guidance.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes az aks get-credentials kubelogin conversion for explicit kubeconfig paths.
Changes:
- Passes the selected kubeconfig path to kubelogin.
- Adds regression coverage for path, environment, merge, and failure scenarios.
- Preserves nonfatal conversion failures.
File summaries
| File | Summary |
|---|---|
src/azure-cli/azure/cli/command_modules/acs/custom.py |
Updates kubelogin invocation; unresolved executable-path security and explicit-path recovery issues remain. |
src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py |
Adds focused regression tests for credential conversion scenarios. |
Review details
Suppressed comments (1)
src/azure-cli/azure/cli/command_modules/acs/custom.py:1956
- For an explicit
--filepath, the fallback hint below still tells users to run conversion without--kubeconfig; with noKUBECONFIGset, kubelogin will operate on~/.kube/configinstead of the file just merged, so the devicecode kubeconfig remains unconverted. Include the effectivepathin this recovery command (and quote it for paths containing spaces).
["kubelogin", "convert-kubeconfig", "-l", "azurecli", "--kubeconfig", path],
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| subprocess.run( | ||
| ["kubelogin", "convert-kubeconfig", "-l", "azurecli"], | ||
| cwd=os.path.dirname(path), | ||
| ["kubelogin", "convert-kubeconfig", "-l", "azurecli", "--kubeconfig", path], |
Member
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Contributor
Live test results —
|
![x-engineering-agent[bot]](https://avatars.githubusercontent.com/in/4205534?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review
Reviewed head b065887220096c3f2471512985302004f9675e74. No confirmed policy or semantic findings in the bounded review targets. Validation is green; the real-converter/recording qualification below remains for the owning squad.
Upstream CI
- 53/53 checks completed successfully; 0 failed, 0 pending, 0 cancelled. No underlying failed ADO builds to classify.
Test validation
- Live test: Passed. Workflow run
- Regression coverage: Present for
acs: 1 focused test file(s) changed.
Scoped review evidence
- All seven review skills were accounted for. Release-artifact, test-strength, user-intent, scope-consistency and domain-edge-case targets were inspected; generated ownership and command/help conventions were not applicable. No direct generated-history edits or unrelated production changes were found.
src/azure-cli/azure/cli/command_modules/acs/custom.py:1951-1958skips conversion for stdout and passes the same resolved kubeconfig path used by the merge, without changing the caller working directory. The title/History Notes describe the path-conversion fix.src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTestverifies serialized file contents, unrelated-file preservation, explicit/relative/environment/default paths, stdout, merge/overwrite failures, missing or failing converters, symlinks and admin credentials. Its child-process converter stand-in makes incorrect path/cwd selection observable.- Optional qualified human-review guidance was unavailable (
ReviewIntelligenceServiceError); the recalled network/billing observations do not establish findings for this ACS change.
Recording and integration qualification
No recording files changed. This patch changes local kubeconfig authentication output and stdout-side-effect behavior, not ARM request construction or service-response mappings. The focused tests use a converter stand-in, so the green workflow does not by itself prove real kubelogin interoperability. Owning-squad attention: verify real-converter output on explicit/relative paths and confirm --file - leaves kubeconfig files untouched; refresh affected scenario recordings only if their expected command/output artifacts change.
Focused regression rerun: pytest -q src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py -k AKSGetCredentialsTest. No tests were executed locally during this review.
Risk assessment
17/100 · Low · High confidence
The Low rating is driven by public CLI behavior.
- Change scope: 2 changed files, 357 changed lines (
+352/-5), including 1 production file. - Affected components:
acs - Risk drivers: public CLI behavior (+18)
- Regression evidence: Changed regression tests are included, reducing risk.
- Confidence: High because changed-line patches were available for every production file.
- Required review: Owning-squad review is recommended for
acsbefore merge.
x-engineering-agent
Bot
added
X Engineering Agent Reviewed
Contributor
|
🔔 Routing this PR to @Azure/act-observability-squad. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 PR Validation — ️✔️ All clear
Related command
az aks get-credentialsDescription
Fixes #34091.
Implemented by X Engineering Agent using Azure AI Foundry. This is an AI-generated change, published by the configured operator.
Testing Guide
az aks get-credentials.src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py.History Notes
[AKS]
az aks get-credentials: Fix kubelogin conversion for explicit kubeconfig pathsThis 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.