Skip to content

{AKS} Preserve resolved executable paths in aks-preview - #10444

Open
FumingZhang wants to merge 5 commits into
Azure:mainfrom
FumingZhang:fix/aks-preview-executable-paths
Open

FumingZhang wants to merge 5 commits into
Azure:mainfrom
FumingZhang:fix/aks-preview-executable-paths

Conversation

@FumingZhang

@FumingZhang FumingZhang commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

{AKS} Preserve resolved executable paths in aks-preview

Summary

  • Use the executable selected during discovery throughout diagnostics, credential conversion, and Bastion commands.
  • Resolve tools from absolute PATH directories, support Windows executable extensions, and preserve explicit executable paths.
  • Preserve argument boundaries for paths containing spaces and select interactive shell behavior by normalized executable basename.
  • Propagate nonzero Bastion tunnel exits after cancelling the subshell, while preserving normal shutdown.
  • Add synthetic regression coverage and a changelog entry.

Related commands

  • az aks kollect
  • az aks kanalyze
  • az aks get-credentials
  • az aks bastion tunnel

Validation

  • Focused helper, diagnostics, credential, and Bastion unit tests: 139 passed, plus 42 subtests.
  • azdev linter --min-severity medium: passed.
  • azdev style aks-preview: pylint and flake8 passed.
  • Changed production modules compiled successfully.
  • python -m build --wheel --no-isolation src/aks-preview: passed.
  • python scripts/ci/test_index.py -q: passed (7 tests passed, 2 skipped).
  • Local credential scans and review of the proposed changes and metadata.
  • Complete extension suite: 1,430 passed, 126 skipped, 95 subtests passed, and one baseline-reproduced app-routing playback failure (see below).

Reviewer notes

  • Empty and relative PATH entries are no longer searched automatically. Explicit executable paths remain supported.
  • Windows path resolution and command construction are covered by platform-simulated unit tests. Native Windows and live Azure end-to-end validation have not been run.
  • Review regressions cover nonzero tunnel exit propagation and cancellation, directory-name collisions, exact shell basenames, Windows executable extensions, and Bash-only terminal restoration.
  • The broader suite's single failure also reproduces against the unmodified parent commit: the installed Key Vault SDK requests API version 2025-05-01, while the app-routing test recording contains 2026-02-01. This unrelated recording/SDK mismatch is not changed by this PR.
  • The change is recorded under Pending; this PR does not publish an extension release.

FumingZhang and others added 2 commits October 6, 2026 23:52
Use the selected executable consistently in diagnostics, credential
conversion, and Bastion. Preserve argument boundaries and shell behavior,
and add regression coverage for executable lookup and error handling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@FumingZhang
FumingZhang marked this pull request as ready for review October 7, 2026 23:25
Copilot AI balanced review requested due to automatic review settings October 7, 2026 23:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Bastion can still suppress non-zero tunnel failures and can misclassify shells based on directory names.

2 open findings
What changed in this PR

Updates AKS Preview commands to consistently reuse resolved executable paths and safely preserve argument boundaries.

Changes:

  • Adds cross-platform executable discovery.
  • Uses resolved paths for diagnostics, credentials, and Bastion subprocesses.
  • Adds regression tests and release notes.
File Description
HISTORY.rst Records executable-path improvements.
_helpers.py Expands executable resolution behavior.
custom.py Reuses the resolved kubelogin path.
aks_diagnostics.py Pins resolved diagnostic tool paths.
bastion/​bastion.py Revises shell and subprocess handling.
test_helpers.py Tests executable discovery.
test_custom.py Tests credential conversion paths.
test_aks_diagnostics.py Tests diagnostic executable handling.
test_aks_bastion.py Tests Bastion subprocess behavior.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread src/aks-preview/azext_aks_preview/bastion/bastion.py
Comment thread src/aks-preview/azext_aks_preview/bastion/bastion.py
FumingZhang and others added 2 commits October 8, 2026 00:00
Address review feedback by reporting nonzero tunnel process exits after cancelling the subshell, while preserving normal cancellation cleanup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review feedback by normalizing the executable basename before selecting shell-specific arguments. Preserve quoted paths and cover directory-name collisions on POSIX and Windows.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@yonzhan

Copy link
Copy Markdown
Collaborator

AKS

@JaysonTaiMicrosoft

Copy link
Copy Markdown

FumingZhang Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants