{AKS} Preserve resolved executable paths in aks-preview

src/aks-preview/HISTORY.rst

@@ -11,6 +11,7 @@ To release a new version, please select a new version number (usually plus 1 to
 
 Pending
 +++++++
+* `az aks kollect`, `az aks kanalyze`, `az aks get-credentials` and `az aks bastion tunnel`: Use resolved executable paths consistently.
 
 22.0.0b10
 +++++++++

src/aks-preview/azext_aks_preview/_helpers.py

@@ -136,17 +136,31 @@ def reset_agentpool_to_name_and_mode(agentpool, mode):
 
 
 def which(binary):
-    path_var = os.getenv('PATH')
-    if platform.system() == 'Windows':
-        binary = binary + '.exe'
-        parts = path_var.split(';')
+    """Resolve an explicit executable path or search absolute directories in PATH."""
+    is_windows = platform.system() == 'Windows'
+    binaries = [binary]
+    if is_windows and not os.path.splitext(binary)[1]:
+        extensions = os.getenv('PATHEXT') or '.COM;.EXE;.BAT;.CMD'
+        binaries = [binary + extension for extension in extensions.split(';') if extension]
+
+    explicit_path = bool(os.path.dirname(binary))
+    if explicit_path:
+        parts = ['']
     else:
-        parts = path_var.split(':')
+        parts = os.getenv('PATH', '').split(';' if is_windows else ':')
 
     for part in parts:
-        bin_path = os.path.join(part, binary)
-        if os.path.exists(bin_path) and os.path.isfile(bin_path) and os.access(bin_path, os.X_OK):
-            return bin_path
+        if is_windows:
+            part = part.strip('"')
+        # Empty, relative and drive-relative entries depend on the working directory.
+        if not explicit_path and (
+            not os.path.isabs(part) or (is_windows and not os.path.splitdrive(part)[0])
+        ):
+            continue
+        for executable in binaries:
+            bin_path = os.path.abspath(os.path.join(part, executable))
+            if os.path.isfile(bin_path) and os.access(bin_path, os.X_OK):
+                return bin_path
 
     return None
 

src/aks-preview/azext_aks_preview/aks_diagnostics.py

@@ -72,7 +72,8 @@ def aks_kollect_cmd(cmd,    # pylint: disable=too-many-statements,too-many-local
 
     mc = client.get(resource_group_name, name)
 
-    if not which('kubectl'):
+    kubectl_path = which('kubectl')
+    if not kubectl_path:
         raise CLIError('Can not find kubectl executable in PATH')
 
     storage_account_id = None
@@ -166,40 +167,40 @@ def aks_kollect_cmd(cmd,    # pylint: disable=too-many-statements,too-many-local
             print()
             print("Cleaning up aks-periscope resources if existing")
 
-            subprocess.call(["kubectl", "--kubeconfig", temp_kubeconfig_path, "delete",
+            subprocess.call([kubectl_path, "--kubeconfig", temp_kubeconfig_path, "delete",
                              "serviceaccount,configmap,daemonset,secret",
                              "--all", "-n", CONST_PERISCOPE_NAMESPACE, "--ignore-not-found"],
                             stderr=subprocess.STDOUT)
 
-            subprocess.call(["kubectl", "--kubeconfig", temp_kubeconfig_path, "delete",
+            subprocess.call([kubectl_path, "--kubeconfig", temp_kubeconfig_path, "delete",
                              "ClusterRoleBinding",
                              "aks-periscope-role-binding", "--ignore-not-found"],
                             stderr=subprocess.STDOUT)
 
-            subprocess.call(["kubectl", "--kubeconfig", temp_kubeconfig_path, "delete",
+            subprocess.call([kubectl_path, "--kubeconfig", temp_kubeconfig_path, "delete",
                              "ClusterRoleBinding",
                              "aks-periscope-role-binding-view", "--ignore-not-found"],
                             stderr=subprocess.STDOUT)
 
-            subprocess.call(["kubectl", "--kubeconfig", temp_kubeconfig_path, "delete",
+            subprocess.call([kubectl_path, "--kubeconfig", temp_kubeconfig_path, "delete",
                              "ClusterRole",
                              "aks-periscope-role", "--ignore-not-found"],
                             stderr=subprocess.STDOUT)
 
-            subprocess.call(["kubectl", "--kubeconfig", temp_kubeconfig_path, "delete",
+            subprocess.call([kubectl_path, "--kubeconfig", temp_kubeconfig_path, "delete",
                              "--all",
                              "apd", "-n", CONST_PERISCOPE_NAMESPACE, "--ignore-not-found"],
                             stderr=subprocess.DEVNULL)
 
-            subprocess.call(["kubectl", "--kubeconfig", temp_kubeconfig_path, "delete",
+            subprocess.call([kubectl_path, "--kubeconfig", temp_kubeconfig_path, "delete",
                              "CustomResourceDefinition",
                              "diagnostics.aks-periscope.azure.github.com", "--ignore-not-found"],
                             stderr=subprocess.STDOUT)
 
             print()
             print("Deploying aks-periscope")
 
-            subprocess.check_output(["kubectl", "--kubeconfig", temp_kubeconfig_path, "apply", "-k",
+            subprocess.check_output([kubectl_path, "--kubeconfig", temp_kubeconfig_path, "apply", "-k",
                                      kustomize_folder, "-n", CONST_PERISCOPE_NAMESPACE], stderr=subprocess.STDOUT)
         except subprocess.CalledProcessError as err:
             raise CLIError(err.output) from err
@@ -249,18 +250,22 @@ def _get_temp_kubeconfig_path(cmd, client, resource_group_name: str, name: str,
 
     if has_aad_profile:
         # The current credentials require interactive login. We need to use kubelogin to update the kubeconfig credential.
-        if not which('kubelogin'):
+        kubelogin_path = which('kubelogin')
+        if not kubelogin_path:
             # No kubelogin found...but we can install it if the user wants.
             if not prompt_y_n('Can not find kubelogin executable in PATH. Install now?', default="y"):
                 # The user doesn't want us to install kubelogin automatically, so we cannot continue.
                 raise CLIError('kubelogin not found. Use az aks install-cli to install.')
 
             # Install kubelogin
             kubelogin_install_location = _get_default_install_location('kubelogin')
-            k8s_install_kubelogin(cmd, 'latest', kubelogin_install_location)
+            if not kubelogin_install_location:
+                raise CLIError('Can not determine kubelogin install location. Use az aks install-cli to install.')
+            kubelogin_path = os.path.abspath(kubelogin_install_location)
+            k8s_install_kubelogin(cmd, 'latest', kubelogin_path)
 
         # kubelogin is installed. Run it to populate user credentials that don't require interactive login.
-        subprocess.check_output(["kubelogin", "convert-kubeconfig", "--kubeconfig", temp_kubeconfig_path, "--login", "azurecli"], stderr=subprocess.STDOUT)
+        subprocess.check_output([kubelogin_path, "convert-kubeconfig", "--kubeconfig", temp_kubeconfig_path, "--login", "azurecli"], stderr=subprocess.STDOUT)
 
     return temp_kubeconfig_path
 
@@ -388,11 +393,12 @@ def _is_windows_hpc_supported(agent_pools):
 
 
 def _display_diagnostics_report(temp_kubeconfig_path):   # pylint: disable=too-many-statements
-    if not which('kubectl'):
+    kubectl_path = which('kubectl')
+    if not kubectl_path:
         raise CLIError('Can not find kubectl executable in PATH')
 
     nodes = subprocess.check_output(
-        ["kubectl", "--kubeconfig", temp_kubeconfig_path,
+        [kubectl_path, "--kubeconfig", temp_kubeconfig_path,
             "get", "node", "--no-headers"],
         universal_newlines=True)
     logger.debug(nodes)
@@ -422,7 +428,7 @@ def _display_diagnostics_report(temp_kubeconfig_path):   # pylint: disable=too-m
     for retry in range(0, max_retry):
         if not apds_created:
             apd = subprocess.check_output(
-                ["kubectl", "--kubeconfig", temp_kubeconfig_path, "get",
+                [kubectl_path, "--kubeconfig", temp_kubeconfig_path, "get",
                     "apd", "-n", CONST_PERISCOPE_NAMESPACE, "--no-headers"],
                 universal_newlines=True
             )
@@ -443,14 +449,14 @@ def _display_diagnostics_report(temp_kubeconfig_path):   # pylint: disable=too-m
                 apdName = "aks-periscope-diagnostic-" + node_name
                 try:
                     network_config = subprocess.check_output(
-                        ["kubectl", "--kubeconfig", temp_kubeconfig_path,
+                        [kubectl_path, "--kubeconfig", temp_kubeconfig_path,
                          "get", "apd", apdName, "-n",
                          CONST_PERISCOPE_NAMESPACE, "-o=jsonpath={.spec.networkconfig}"],
                         universal_newlines=True)
                     logger.debug('Dns status for node %s is %s',
                                  node_name, network_config)
                     network_status = subprocess.check_output(
-                        ["kubectl", "--kubeconfig", temp_kubeconfig_path,
+                        [kubectl_path, "--kubeconfig", temp_kubeconfig_path,
                          "get", "apd", apdName, "-n",
                          CONST_PERISCOPE_NAMESPACE, "-o=jsonpath={.spec.networkoutbound}"],
                         universal_newlines=True)

src/aks-preview/azext_aks_preview/bastion/bastion.py

@@ -5,7 +5,7 @@
 
 import asyncio
 import os
-import shutil
+import shlex
 import signal
 import socket
 import subprocess
@@ -15,6 +15,7 @@
 
 import psutil
 import yaml
+from azext_aks_preview._helpers import which
 from azure.cli.command_modules.acs._consts import DecoratorEarlyExitException
 from azure.cli.core.azclierror import (CLIInternalError,
                                        InvalidArgumentValueError,
@@ -276,25 +277,27 @@ async def aks_bastion_runner(
     else:
         task2 = asyncio.create_task(_aks_bastion_launch_subshell(kubeconfig_path, port))
 
-    _, pending = await asyncio.wait([task1, task2], return_when=asyncio.FIRST_COMPLETED)
+    done, pending = await asyncio.wait([task1, task2], return_when=asyncio.FIRST_COMPLETED)
 
     for task in pending:
         task.cancel()
 
     # Wait for the cancellations to finish
     await asyncio.gather(*pending, return_exceptions=True)
+    await asyncio.gather(*done)
 
 
 def aks_batsion_clean_up():
     pass
 
 
 def _aks_bastion_get_az_cmd_name():
-    """Get the name of the az command based on system platform."""
+    """Resolve the az command for the current platform."""
 
-    if sys.platform.startswith("win"):
-        return "az.cmd"
-    return "az"
+    executable = which("az.cmd" if sys.platform.startswith("win") else "az")
+    if not executable:
+        raise CLIInternalError("Cannot find Azure CLI executable in PATH.")
+    return executable
 
 
 def _aks_bastion_get_current_shell_cmd():
@@ -380,42 +383,48 @@ def _get_powershell_executable(grandparent):
 
 def _get_powershell_executable_from_path():
     """Try to find PowerShell executable in PATH, preferring pwsh over powershell."""
-    pwsh_path = shutil.which("pwsh")
+    pwsh_path = which("pwsh")
     if pwsh_path:
         logger.debug("Found pwsh at: %s", pwsh_path)
-        return "pwsh"
+        return pwsh_path
 
-    powershell_path = shutil.which("powershell")
+    powershell_path = which("powershell")
     if powershell_path:
         logger.debug("Found powershell at: %s", powershell_path)
-        return "powershell"
+        return powershell_path
 
     return None
 
 
 def _aks_bastion_prepare_shell_cmd(kubeconfig_path):
     """Prepare the shell command to launch a subshell with KUBECONFIG set."""
 
-    shell_cmd = _aks_bastion_get_current_shell_cmd()
-    updated_shell_cmd = shell_cmd
+    detected_shell = _aks_bastion_get_current_shell_cmd()
+    shell_cmd = which(detected_shell)
+    if not shell_cmd:
+        raise CLIInternalError(f"Cannot find shell executable '{detected_shell}' in PATH.")
+    is_windows = sys.platform.startswith("win")
+    quoted_shell_cmd = f'"{shell_cmd}"' if is_windows else shlex.quote(shell_cmd)
+    updated_shell_cmd = quoted_shell_cmd
 
     # Handle different shell types
     if shell_cmd.endswith("bash") and os.path.exists(os.path.expanduser("~/.bashrc")):
-        updated_shell_cmd = (
-            f"""{shell_cmd} -c '{shell_cmd} --rcfile <(cat ~/.bashrc; """
-            f"""echo "export KUBECONFIG={kubeconfig_path}")'"""
+        export_command = f"export KUBECONFIG={shlex.quote(kubeconfig_path)}"
+        bash_command = (
+            f"{quoted_shell_cmd} --rcfile <(cat ~/.bashrc; "
+            f"printf '%s\\n' {shlex.quote(export_command)})"
         )
+        updated_shell_cmd = f"{quoted_shell_cmd} -c {shlex.quote(bash_command)}"
     elif shell_cmd in ["pwsh", "powershell"] or "pwsh" in shell_cmd.lower() or "powershell" in shell_cmd.lower():
         # PowerShell: Set environment variable and start new session
         # Use proper PowerShell syntax for setting environment variables
         escaped_path = kubeconfig_path.replace("'", "''")  # Escape single quotes for PowerShell
-        if shell_cmd == "pwsh" or "pwsh" in shell_cmd.lower():
-            updated_shell_cmd = f'pwsh -NoExit -Command "$env:KUBECONFIG=\'{escaped_path}\'"'
-        else:
-            updated_shell_cmd = f'powershell -NoExit -Command "$env:KUBECONFIG=\'{escaped_path}\'"'
+        powershell_command = f"$env:KUBECONFIG='{escaped_path}'"
+        quoted_command = f'"{powershell_command}"' if is_windows else shlex.quote(powershell_command)
+        updated_shell_cmd = f"{quoted_shell_cmd} -NoExit -Command {quoted_command}"
     elif shell_cmd == "cmd" or "cmd" in shell_cmd.lower():
         # CMD: Set environment variable and keep session open
-        updated_shell_cmd = f'cmd /k "set KUBECONFIG={kubeconfig_path}"'
+        updated_shell_cmd = f'{quoted_shell_cmd} /k set "KUBECONFIG={kubeconfig_path}"'
 
     return shell_cmd, updated_shell_cmd
 
@@ -443,6 +452,11 @@ async def _aks_bastion_launch_subshell(kubeconfig_path, port):
         env = os.environ.copy()
         env.update({"KUBECONFIG": kubeconfig_path})
         shell_cmd, updated_shell_cmd = _aks_bastion_prepare_shell_cmd(kubeconfig_path)
+        shell_executable = None
+        if sys.platform.startswith("win"):
+            shell_executable = which("cmd.exe")
+            if not shell_executable:
+                raise CLIInternalError("Cannot find command processor executable 'cmd.exe' in PATH.")
         logger.warning(
             "Launching subshell with command '%s'. Setting env var KUBECONFIG to '%s'.",
             updated_shell_cmd,
@@ -458,6 +472,7 @@ async def _aks_bastion_launch_subshell(kubeconfig_path, port):
             stderr=None,
             shell=True,
             env=env,
+            executable=shell_executable,
         )
         logger.info("Subshell launched with PID: %s", subshell_process.pid)
 
@@ -493,22 +508,24 @@ async def _aks_bastion_launch_tunnel(bastion_resource, port, mc_id, subscription
     tunnel_proces = None
     try:
         az_cmd_name = _aks_bastion_get_az_cmd_name()
-        cmd = (
-            f"{az_cmd_name} network bastion tunnel --resource-group {bastion_resource.resource_group} "
-            f"--name {bastion_resource.name} --port {port} --target-resource-id {mc_id} --resource-port 443"
-        )
+        cmd = [
+            az_cmd_name, "network", "bastion", "tunnel",
+            "--resource-group", bastion_resource.resource_group,
+            "--name", bastion_resource.name, "--port", str(port),
+            "--target-resource-id", mc_id, "--resource-port", "443",
+        ]
         # the bastion may live in a different subscription than the cluster; prefer the
         # subscription resolved from the bastion resource over the cluster subscription
         bastion_subscription_id = getattr(bastion_resource, "subscription", None) or subscription_id
         if bastion_subscription_id:
-            cmd += f" --subscription {bastion_subscription_id}"
-        logger.warning("Creating bastion tunnel with command: '%s'", cmd)
+            cmd.extend(["--subscription", bastion_subscription_id])
+        logger.warning("Creating bastion tunnel with command: '%s'", subprocess.list2cmdline(cmd))
 
         # Use start_new_session on Unix to create a new process group
         # This allows us to kill the entire process tree when cleaning up
         start_new_session = not sys.platform.startswith("win")
         tunnel_proces = await asyncio.create_subprocess_exec(
-            *(cmd.split()),
+            *cmd,
             stdin=asyncio.subprocess.DEVNULL,
             stdout=asyncio.subprocess.DEVNULL,
             stderr=asyncio.subprocess.DEVNULL,
@@ -564,8 +581,13 @@ def _aks_bastion_kill_process_tree(process):
     if sys.platform.startswith("win"):
         # On Windows, use taskkill with /T flag to kill the process tree
         try:
+            taskkill_path = which("taskkill.exe")
+            if not taskkill_path:
+                logger.warning("Cannot find taskkill.exe in PATH. Terminating the tunnel process directly.")
+                process.terminate()
+                return
             subprocess.run(
-                ["taskkill", "/T", "/F", "/PID", str(pid)],
+                [taskkill_path, "/T", "/F", "/PID", str(pid)],
                 capture_output=True,
                 check=False,
             )
@@ -616,12 +638,15 @@ async def _aks_bastion_test_hook(kubeconfig_path, port, kubectl_path):
     """Test hook to validate the bastion tunnel and run a kubectl command."""
     if not await _aks_bastion_validate_tunnel(port):
         raise CLIInternalError(f"Bastion tunnel failed to set up on port {port}.")
-    kubectl_process = await asyncio.create_subprocess_shell(
-        f"{kubectl_path} --kubeconfig {kubeconfig_path} get nodes",
+    executable = which(kubectl_path)
+    if not executable:
+        raise CLIInternalError(f"Cannot find kubectl executable '{kubectl_path}'.")
+    kubectl_process = await asyncio.create_subprocess_exec(
+        executable, "--kubeconfig", kubeconfig_path, "get", "nodes",
         stdin=asyncio.subprocess.DEVNULL,
         stdout=asyncio.subprocess.DEVNULL,
         stderr=asyncio.subprocess.DEVNULL,
-        shell=True,
+        shell=False,
     )
     await asyncio.wait_for(kubectl_process.wait(), timeout=10)
     if kubectl_process.returncode != 0:

src/aks-preview/azext_aks_preview/custom.py

@@ -1930,11 +1930,12 @@ def aks_get_credentials(
             path, kubeconfig, overwrite_existing, context_name)
         # Check if kubeconfig requires kubelogin with devicecode and convert it
         if uses_kubelogin_devicecode(kubeconfig):
-            if which("kubelogin"):
+            kubelogin_path = which("kubelogin")
+            if kubelogin_path:
                 try:
                     # Run kubelogin convert-kubeconfig -l azurecli
                     subprocess.run(
-                        ["kubelogin", "convert-kubeconfig", "-l", "azurecli"],
+                        [kubelogin_path, "convert-kubeconfig", "-l", "azurecli"],
                         cwd=os.path.dirname(path),
                         check=True,
                     )