[AKS] Fix #34091: az aks get-credentials: Fix kubelogin conversion for explicit kubeconfig paths

[Aditya Pujara (a0x1ab)]

🤖 PR Validation — ️✔️ All clear

Breaking Changes	Tests
️✔️ None	️✔️ 130/130

Related command

az aks get-credentials

Description

Fixes #34091.

Implemented by X Engineering Agent using Azure AI Foundry. This is an AI-generated change, published by the configured operator.

Testing Guide

• Run az aks get-credentials.
• Focused regression coverage: src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py.
• Repository CI and the authorized live-test workflow validate the published revision.

History Notes

[AKS] az aks get-credentials: Fix kubelogin conversion for explicit kubeconfig paths

---

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.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

🟡 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 --file path, the fallback hint below still tells users to run conversion without --kubeconfig; with no KUBECONFIG set, kubelogin will operate on ~/.kube/config instead of the file just merged, so the devicecode kubeconfig remains unconverted. Include the effective path in 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.

[FumingZhang]

Aditya Pujara (@a0x1ab) This is a valid Windows executable-search issue and is worth addressing in this PR. The local which() helper searches PATH, but its result is discarded; launching the bare kubelogin name lets Windows CreateProcess search the caller’s current directory before PATH, so it may execute a different kubelogin.exe than the one that was checked. Please retain the resolved value and invoke os.path.abspath(kubelogin_path), matching the existing kubectl hardening in this module. This is compatible with the #34091 fix: keep cwd unset and continue passing --kubeconfig, path.

[Aditya Pujara (a0x1ab)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).

[microsoft-github-policy-service]

🔔 Routing this PR to @Azure/act-observability-squad.

[microsoft-github-policy-service]

🔔 Routing this PR to @Azure/act-observability-squad.

[Yong Zhang (yonzhan)]

Please fix CI issues

[x-engineering-agent]

Live test results — azdev test --live --series (changed test files only)

✅ PASS

Selectors: acs.AKSGetCredentialsTest (module)
PR head ref: agent-assist/azure-azure-cli-issue-34091-5cea4d61c040
PR head sha: 42bcbf4e3e35b3ade81bafb0bad46de76134c523
PR base ref: dev
New test files in PR: false

Changed test files run

src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py

Workflow run: https://github.com/Azure/issue-sentinel/actions/runs/38495862107

Last 80 lines of azdev output

=============
| Run Tests |
=============


test index found: /home/runner/.azdev/env_config/home/runner/work/issue-sentinel/issue-sentinel/.venv/test_index/latest.json

Test on modules: acs.AKSGetCredentialsTest

WARNING: RUNNING TESTS LIVE
The tests are set to run against current profile "latest"
============================= test session starts ==============================
platform linux -- Python 3.12.15, pytest-9.1.1, pluggy-1.6.0 -- /home/runner/work/issue-sentinel/issue-sentinel/.venv/bin/python
cachedir: .pytest_cache
rootdir: /home/runner/work/issue-sentinel/issue-sentinel/azure-cli/src/azure-cli
configfile: pyproject.toml
plugins: platformdirs-4.13.0, forked-1.7.5, xdist-3.8.0
collecting ... collected 21 items

azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_absolute_filename PASSED [  4%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_admin_does_not_convert PASSED [  9%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_bare_filename PASSED [ 14%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_context_name PASSED [ 19%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_converter_nonzero_exit_is_nonfatal PASSED [ 23%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_converter_startup_error_is_nonfatal PASSED [ 28%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_default_file PASSED [ 33%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_dot_relative_filename PASSED [ 38%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_empty_first_environment_entry PASSED [ 42%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_explicit_file_overrides_environment PASSED [ 47%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_first_environment_entry PASSED [ 52%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_literal_tilde_path PASSED [ 57%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_merge_failure_does_not_convert PASSED [ 61%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_missing_kubelogin_is_nonfatal PASSED [ 66%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_nested_relative_filename PASSED [ 71%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_non_devicecode PASSED [ 76%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_overwrite_existing PASSED [ 80%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_path_with_spaces PASSED [ 85%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_relative_environment_entry PASSED [ 90%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_stdout_skips_conversion PASSED [ 95%]
azure-cli/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py::AKSGetCredentialsTest::test_aks_get_credentials_symlink_does_not_convert PASSED [100%]

- generated xml file: /home/runner/work/issue-sentinel/issue-sentinel/test-output/results.xml -
============================== 21 passed in 1.45s ==============================

Posted by the X Engineering Agent live-test workflow.

[x-engineering-agent]

Aditya Pujara (@a0x1ab)

Review

The explicit kubeconfig-path fix addresses the reported empty working-directory failure, but executable selection still needs correction before human approval. All 52 upstream checks and the Agent live-test run passed on 42bcbf4e3e35b3ade81bafb0bad46de76134c523; the remaining finding is confirmed from the changed invocation, not a CI failure.

Domain edge case: invoke the resolved executable

At src/azure-cli/azure/cli/command_modules/acs/custom.py:2066-2071, which("kubelogin") is used only as a Boolean, while the changed subprocess argument at line 2070 is still the bare name kubelogin. Removing cwd makes the caller's working directory the invocation context. On Windows, CreateProcess can select a current-directory kubelogin.exe before the executable found on PATH, exposing the selected kubeconfig to an unintended process. The same file already documents and avoids this behavior for kubectl at lines 361-367 and 2339-2345.

Store the result of which, normalize a successful result with os.path.abspath(), and use that resolved path as argv[0]. Preserve --kubeconfig path, caller-relative kubeconfig semantics, stdout handling, and the existing nonfatal missing/converter-error behavior.

Update src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py: the new _run_kubelogin helper replaces argv[0] with sys.executable, and the assertions currently expect the bare name, so those tests cannot detect executable-selection regressions. Return a distinct resolved path from which, assert its absolute form is passed to subprocess, and cover a relative PATH result and Windows current-directory shadowing with a benign fixture. Keep the existing file-content, unrelated-config, and failure-path assertions. Run python -m unittest azure.cli.command_modules.acs.tests.latest.test_custom.AKSGetCredentialsTest in the development environment on Windows and POSIX, then rerun upstream validation and the authorized live-test workflow for the new head.

Test validation

• Live test: Passed. Workflow run
• Regression coverage: For acs: 1 focused test file(s) changed. Scenario coverage is unverified; review the issue's required conditions, test setup, and assertions before approval. No linked issue was found in the PR timeline; inspect the source issue and its required scenario manually.

I inspected source issue #34091 through the sanitized issue view. The added AKSGetCredentialsTest cases exercise bare, relative, absolute, spaced, and environment-selected kubeconfig paths, and assert the merged YAML uses azurecli without modifying unrelated configurations. Those are changed-test assertions, separate from the Agent workflow result. The child-interpreter converter is a stand-in, not evidence that the real kubelogin executable was selected safely; add the focused executable-path regression above before re-review. No service request/response fields change, so these filesystem assertions do not require an ARM recording update.

Risk assessment

17/100 · Low · High confidence

• Change scope: 2 changed files, 354 changed lines (+349 / -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 acs before merge.

Posted by x-engineering-agent (Reviewer)

[FumingZhang]

Azure/azure-cli#34099 and Azure/azure-cli#34104 implement the same runtime fix; only comments and tests differ. I suggest consolidating into #34099 for its broader test coverage, carrying over useful cases from #34104. Before merging, please also address the executable-resolution feedback: invoke the absolute path returned by which (normalized with os.path.abspath) instead of bare kubelogin, while keeping --kubeconfig path and no cwd override.

[x-engineering-agent]

X Engineering Agent Report

⏳ In progress — X Engineering Agent is reviewing this PR at 42bcbf4. This comment will be updated with the results.