diff --git a/src/ssh/azext_ssh/_help.py b/src/ssh/azext_ssh/_help.py index 2bda42be1b9..9f4dac63242 100644 --- a/src/ssh/azext_ssh/_help.py +++ b/src/ssh/azext_ssh/_help.py @@ -13,13 +13,13 @@ helps['ssh vm'] = """ type: command short-summary: SSH into Azure VMs or Arc Servers. - long-summary: Users can login using AAD issued certificates or using local user credentials. We recommend login using AAD issued certificates. To SSH as a local user in the target machine, you must provide the local user name using the --local-user argument. + long-summary: Users can login using AAD issued certificates or using local user credentials. We recommend login using AAD issued certificates. To SSH using local user credentials, you must provide the local user name using the --local-user parameter. examples: - - name: Give a resource group and VM to SSH using AAD issued certificates + - name: Give a resource group name and machine name to SSH using AAD issued certificates text: | - az ssh vm --resource-group myResourceGroup --vm-name myVM + az ssh vm --resource-group myResourceGroup --name myVM - - name: Give the public IP (or hostname) of a VM to SSH to SSH using AAD issued certificates + - name: Give the public IP (or hostname) of a VM to SSH using AAD issued certificates text: | az ssh vm --ip 1.2.3.4 az ssh vm --hostname example.com @@ -32,37 +32,51 @@ text: | az ssh vm --ip 1.2.3.4 -- -A -o ForwardX11=yes - - name: Give the Resource Type of a VM to SSH using AAD issued certificates. Using the resource type is useful when there is an Azure VM and a Arc Server with the same name in the same resource group. + - name: Give the Resource Type of the target. Useful when there is an Azure VM and an Arc Server with the same name in the same resource group. Resource type can be either "Microsoft.HybridCompute" for Arc Servers or "Microsoft.Compute" for Azure Virtual Machines. text: | - az ssh vm --resource-type Microsoft.Compute --resource-group myResourceGroup --vm-name myVM + az ssh vm --resource-type [Microsoft.Compute|Microsoft.HybridCompute] --resource-group myResourceGroup --name myVM - - name: Give a local user name to SSH using local user credentials on the target machine using certificate based authentication. + - name: Give a local user name to SSH with local user credentials using certificate based authentication. text: | az ssh vm --local-user username --ip 1.2.3.4 --certificate-file cert.pub --private-key key - - name: Give a local user name to SSH using local user credentials on the target machine using key based authentication. + - name: Give a local user name to SSH with local user credentials using key based authentication. text: | - az ssh vm --local-user username --resource-group myResourceGroup --vm-name myVM --private-key-file key + az ssh vm --local-user username --resource-group myResourceGroup --name myVM --private-key-file key - - name: Give a local user name to SSH using local user credentials on the target machine using password based authentication. + - name: Give a local user name to SSH with local user credentials using password based authentication. text: | - az ssh vm --local-user username --resource-group myResourceGroup --vm-name myArcServer + az ssh vm --local-user username --resource-group myResourceGroup --name myArcServer + + - name: Give a SSH Client Folder to use the ssh executables in that folder, like ssh-keygen.exe and ssh.exe. If not provided, the extension attempt to use pre-installed OpenSSH client (in that case, ensure OpenSSH client is installed and the Path environment variable is set correctly). + text: | + az ssh vm --resource-group myResourceGroup --name myVM --ssh-client-folder "C:\\Program Files\\OpenSSH" """ helps['ssh config'] = """ type: command - short-summary: Create an SSH config for resources (Azure VMs, etc) which can then be used by clients that support OpenSSH configs and certificates - long-summary: Other software (git/rsync/etc) that support setting an SSH command can be set to use the config file by setting the command to 'ssh -F /path/to/config' e.g. rsync -e 'ssh -F /path/to/config' + short-summary: Create an SSH config for resources (Azure VMs, Arc Servers, etc) which can then be used by clients that support OpenSSH configs and certificates + long-summary: Other software (git/rsync/etc) that support setting an SSH command can be set to use the config file by setting the command to 'ssh -F /path/to/config' e.g. rsync -e 'ssh -F /path/to/config'. Users can create ssh config files that use AAD issued certificates or local user credentials. examples: - - name: Give a resource group and VM for which to create a config, and save in a local file + - name: Give the resource group and machine name for which to create a config using AAD issued certificates, save in a local file, and then ssh into that resource text: | - az ssh config --resource-group myResourceGroup --vm-name myVm --file ./sshconfig + az ssh config --resource-group myResourceGroup --name myVm --file ./sshconfig + ssh -F ./sshconfig myResourceGroup-myVM - - name: Give the public IP (or hostname) of a VM for which to create a config and then ssh + - name: Give the public IP (or hostname) of an Azure VM for which to create a config and then ssh into that VM text: | az ssh config --ip 1.2.3.4 --file ./sshconfig ssh -F ./sshconfig 1.2.3.4 + - name: Give a local user to create a config using local user credentials, save in local file, and then ssh into that resource + text: | + az ssh config --resource-group myResourceGroup --name myMachine --local-user username --certificate-file cert --private-key-file key --file ./sshconfig + ssh -F ./sshconfig MyResourceGroup-myMachine-username + + - name: Give Keys Destination Folder to store the generated keys and certificates. If not provided, SSH keys are stored in new folder "az_ssh_config" next to the config file. + text: | + az ssh config --ip 1.2.3.4 --file ./sshconfig --keys-destination-folder /home/user/mykeys + - name: Create a generic config for use with any host text: | #Bash @@ -76,6 +90,14 @@ az ssh config --ip \\* --file ./sshconfig rsync -e 'ssh -F ./sshconfig' -avP directory/ myvm:~/directory GIT_SSH_COMMAND="ssh -F ./sshconfig" git clone myvm:~/gitrepo + + - name: Give SSH Client Folder to use the ssh executables in that folder. If not provided, the extension attempt to use pre-installed OpenSSH client (in that case, ensure OpenSSH client is installed and the Path environment variable is set correctly). + text: | + az ssh vm --resource-group myResourceGroup --name myMachine --ssh-client-folder "C:\\Program Files\\OpenSSH" + + - name: Give the Resource Type of the target. Useful when there is an Azure VM and an Arc Server with the same name in the same resource group. Resource type can be either "Microsoft.HybridCompute" for Arc Servers or "Microsoft.Compute" for Azure Virtual Machines. + text: | + az ssh config --resource-type [Microsoft.Compute|Microsoft.HybridCompute] --resource-group myResourceGroup --name myVM --file ./myconfig """ helps['ssh cert'] = """ @@ -90,25 +112,33 @@ helps['ssh arc'] = """ type: command short-summary: SSH into Azure Arc Servers - long-summary: Users can now login using AAD issued certificates or using local user credentials. We recommend login using AAD issued certificates as azure automatically rotate SSH CA keys. To SSH as a local user in the target machine, you must provide the local user name using the --local-user argument. + long-summary: Users can login using AAD issued certificates or using local user credentials. We recommend login using AAD issued certificates as azure automatically rotate SSH CA keys. To SSH as a local user in the target machine, you must provide the local user name using the --local-user argument. examples: - - name: Give a resource group and Arc Server Name to SSH using AAD issued certificates + - name: Give a resource group name and machine name to SSH using AAD issued certificates text: | - az ssh arc --resource-group myResourceGroup --vm-name myArcServer + az ssh vm --resource-group myResourceGroup --name myMachine - name: Using a custom private key file text: | - az ssh arc --resource-group myResourceGroup --vm-name myArcServer --private-key-file key --public-key-file key.pub + az ssh vm --resource-group myResourceGroup --name myMachine --private-key-file key --public-key-file key.pub + + - name: Using additional ssh arguments + text: | + az ssh vm --resource-group myResourceGroup --name myMachine -- -A -o ForwardX11=yes + + - name: Give a local user name to SSH with local user credentials using certificate based authentication. + text: | + az ssh vm --local-user username --resource-group myResourceGroup --name myMachine --certificate-file cert.pub --private-key key - - name: Give a local user name to SSH to a local user using certificate-based authentication + - name: Give a local user name to SSH with local user credentials using key based authentication. text: | - az ssh arc --resource-group myResourceGroup --vm-name myArcServer --certificate-file cert.pub --private-key key --local-user name + az ssh vm --local-user username --resource-group myResourceGroup --name myMachine --private-key-file key - - name: Give a local user name to SSH to a local user using key-based authentication + - name: Give a local user name to SSH with local user credentials using password based authentication. text: | - az ssh arc --resource-group myRG --vm-name myVM --local-user name --private-key-file key + az ssh vm --local-user username --resource-group myResourceGroup --name myMachine - - name: Give a local user name to SSH to a local user using password-based authentication + - name: Give a SSH Client Folder to use the ssh executables in that folder, like ssh-keygen.exe and ssh.exe. If not provided, the extension attempt to use pre-installed OpenSSH client (ensure OpenSSH client is installed and the Path environment variable is set correctly). text: | - az ssh arc --resource-group myResourceGroup --vm-name myArcServer --local-user username + az ssh vm --resource-group myResourceGroup --name myMachine --ssh-client-folder "C:\\Program Files\\OpenSSH" """ diff --git a/src/ssh/azext_ssh/connectivity_utils.py b/src/ssh/azext_ssh/connectivity_utils.py index da34fa39674..a7f0cf5bc37 100644 --- a/src/ssh/azext_ssh/connectivity_utils.py +++ b/src/ssh/azext_ssh/connectivity_utils.py @@ -53,14 +53,8 @@ def get_relay_information(cmd, resource_group, vm_name, certificate_validity_in_ time_elapsed = time.time() - t0 telemetry.add_extension_event('ssh', {'Context.Default.AzureCLI.SSHListCredentialsTime': time_elapsed}) except Exception as e: - telemetry.set_exception(exception='Call to listCredentials failed', - fault_type=consts.LIST_CREDENTIALS_FAILED_FAULT_TYPE, - summary=f'listCredentials failed with error: {str(e)}.') raise azclierror.ClientRequestError(f"Request for Azure Relay Information Failed:\n{str(e)}") except Exception as e: - telemetry.set_exception(exception='Call to listCredentials failed', - fault_type=consts.LIST_CREDENTIALS_FAILED_FAULT_TYPE, - summary=f'listCredentials failed with error: {str(e)}.') raise azclierror.ClientRequestError(f"Request for Azure Relay Information Failed:\n{str(e)}") return result @@ -91,8 +85,6 @@ def get_client_side_proxy(arc_proxy_folder): response_content = response.read() response.close() except Exception as e: - telemetry.set_exception(exception=e, fault_type=consts.PROXY_DOWNLOAD_FAILED_FAULT_TYPE, - summary=f'Failed to download proxy from {request_uri}') raise azclierror.ClientRequestError(f"Failed to download client proxy executable from {request_uri}. " "Error: " + str(e)) from e time_elapsed = time.time() - t0 @@ -136,9 +128,6 @@ def _get_proxy_filename_and_url(arc_proxy_folder): elif machine == '': raise azclierror.BadRequestError("Couldn't identify the platform architecture.") else: - telemetry.set_exception(exception='Unsuported architecture for installing proxy', - fault_type=consts.PROXY_UNSUPPORTED_ARCH_FAULT_TYPE, - summary=f'{machine} is not supported for installing client proxy') raise azclierror.BadRequestError(f"Unsuported architecture: {machine} is not currently supported") # define the request url and install location based on the os and architecture @@ -153,9 +142,6 @@ def _get_proxy_filename_and_url(arc_proxy_folder): install_location = install_location + ".exe" older_location = older_location + ".exe" elif operating_system not in ('Linux', 'Darwin'): - telemetry.set_exception(exception='Unsuported OS for installing ssh client proxy', - fault_type=consts.PROXY_UNSUPPORTED_OS_FAULT_TYPE, - summary=f'{operating_system} is not supported for installing client proxy') raise azclierror.BadRequestError(f"Unsuported OS: {operating_system} platform is not currently supported") if not arc_proxy_folder: diff --git a/src/ssh/azext_ssh/constants.py b/src/ssh/azext_ssh/constants.py index a1e03590450..92c56bddfdf 100644 --- a/src/ssh/azext_ssh/constants.py +++ b/src/ssh/azext_ssh/constants.py @@ -10,8 +10,7 @@ CLEANUP_TIME_INTERVAL_IN_SECONDS = 10 CLEANUP_AWAIT_TERMINATION_IN_SECONDS = 30 RELAY_INFO_MAXIMUM_DURATION_IN_SECONDS = 3600 -PROXY_UNSUPPORTED_ARCH_FAULT_TYPE = 'client-proxy-unsupported-architecture-error' -PROXY_UNSUPPORTED_OS_FAULT_TYPE = 'client-proxy-unsupported-os-error' -PROXY_DOWNLOAD_FAILED_FAULT_TYPE = 'client-proxy-download-failed-error' -LIST_CREDENTIALS_FAILED_FAULT_TYPE = 'get-relay-information-failed-error' WINDOWS_INVALID_FOLDERNAME_CHARS = "\\/*:<>?\"|" +RECOMMENDATION_SSH_CLIENT_NOT_FOUND = ("Ensure OpenSSH is installed and the PATH Environment " + "Variable is set correctly.\nAlternatively, use " + "--ssh-client-folder to provide OpenSSH folder path.") diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index 98a2160f656..30b3258ec94 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -412,6 +412,9 @@ def _decide_resource_type(cmd, op_info): raise azclierror.RequiredArgumentMissingError("SSH Login to AAD user is not currently supported for Windows. " "Please provide --local-user.") + if os_type: + telemetry.add_extension_event('ssh', {'Context.Default.AzureCLI.TargetOSType': os_type}) + if is_arc_server: return "Microsoft.HybridCompute" return "Microsoft.Compute" diff --git a/src/ssh/azext_ssh/ssh_info.py b/src/ssh/azext_ssh/ssh_info.py index 6bccf8dd7fe..5e99459e924 100644 --- a/src/ssh/azext_ssh/ssh_info.py +++ b/src/ssh/azext_ssh/ssh_info.py @@ -116,7 +116,7 @@ def get_config_text(self, is_aad): # default to all hosts for config if not self.ip: self.ip = "*" - lines = lines + self._get_ip_entry() + lines = lines + self._get_ip_entry(is_aad) return lines def _get_arc_entry(self, is_aad): @@ -154,9 +154,13 @@ def _get_rg_and_vm_entry(self, is_aad): lines.append("\tPort " + self.port) return lines - def _get_ip_entry(self): + def _get_ip_entry(self, is_aad): lines = [] - lines.append("Host " + self.ip) + if is_aad: + lines.append("Host " + self.ip) + else: + lines.append("Host " + self.ip + "-" + self.local_user) + lines.append("\tHostName " + self.ip) lines.append("\tUser " + self.local_user) if self.cert_file: lines.append("\tCertificateFile \"" + self.cert_file + "\"") diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index d8be733f3a6..a123df04dc0 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -55,11 +55,21 @@ def start_ssh_connection(op_info, delete_keys, delete_cert): connection_duration = time.time() logger.debug("Running ssh command %s", ' '.join(command)) - connection_status = subprocess.call(command, shell=platform.system() == 'Windows', env=env) + + # pylint: disable=subprocess-run-check + try: + if set(['-v', '-vv', '-vvv']).isdisjoint(ssh_arg_list) or log_file: + connection_status = subprocess.run(command, env=env, stderr=subprocess.PIPE, text=True) + else: + # Logs are sent to stderr. In that case, we shouldn't capture stderr. + connection_status = subprocess.run(command, env=env, text=True) + except Exception as e: + raise azclierror.BadRequestError(f"Failed to run ssh command with error: {str(e)}.", + const.RECOMMENDATION_SSH_CLIENT_NOT_FOUND) connection_duration = (time.time() - connection_duration) / 60 ssh_connection_data = {'Context.Default.AzureCLI.SSHConnectionDurationInMinutes': connection_duration} - if _get_connection_status(log_file, connection_status): + if connection_status and connection_status.returncode == 0: ssh_connection_data['Context.Default.AzureCLI.SSHConnectionStatus'] = "Success" telemetry.add_extension_event('ssh', ssh_connection_data) @@ -88,14 +98,22 @@ def create_ssh_keyfile(private_key_file, ssh_client_folder=None): sshkeygen_path = _get_ssh_client_path("ssh-keygen", ssh_client_folder) command = [sshkeygen_path, "-f", private_key_file, "-t", "rsa", "-q", "-N", ""] logger.debug("Running ssh-keygen command %s", ' '.join(command)) - subprocess.call(command, shell=platform.system() == 'Windows') + try: + subprocess.call(command) + except Exception as e: + raise azclierror.BadRequestError(f"Failed to create ssh key file with error: {str(e)}.", + const.RECOMMENDATION_SSH_CLIENT_NOT_FOUND) def get_ssh_cert_info(cert_file, ssh_client_folder=None): sshkeygen_path = _get_ssh_client_path("ssh-keygen", ssh_client_folder) command = [sshkeygen_path, "-L", "-f", cert_file] logger.debug("Running ssh-keygen command %s", ' '.join(command)) - return subprocess.check_output(command, shell=platform.system() == 'Windows').decode().splitlines() + try: + return subprocess.check_output(command).decode().splitlines() + except Exception as e: + raise azclierror.BadRequestError(f"Failed to get certificate info with error: {str(e)}.", + const.RECOMMENDATION_SSH_CLIENT_NOT_FOUND) def _get_ssh_cert_validity(cert_file, ssh_client_folder=None): @@ -148,7 +166,7 @@ def _print_error_messages_from_ssh_log(log_file, connection_status, delete_cert) log_lines = log_text.splitlines() if ("debug1: Authentication succeeded" not in log_text and not re.search("^Authenticated to .*\n", log_text, re.MULTILINE)) \ - or connection_status: + or (connection_status and connection_status.returncode): for line in log_lines: if "debug1:" not in line: print(line) @@ -198,48 +216,9 @@ def _get_ssh_client_path(ssh_command="ssh", ssh_client_folder=None): logger.debug("Attempting to run %s from path %s", ssh_command, ssh_path) return ssh_path logger.warning("Could not find %s in provided --ssh-client-folder %s. " - "Attempting to get pre-installed OpenSSH bits.", ssh_path, ssh_client_folder) + "Attempting to use pre-installed OpenSSH.", ssh_path, ssh_client_folder) ssh_path = ssh_command - - if platform.system() == 'Windows': - # If OS architecture is 64bit and python architecture is 32bit, - # look for System32 under SysNative folder. - machine = platform.machine() - os_architecture = None - # python interpreter architecture - platform_architecture = platform.architecture()[0] - sys_path = None - - if machine.endswith('64'): - os_architecture = '64bit' - elif machine.endswith('86'): - os_architecture = '32bit' - elif machine == '': - raise azclierror.BadRequestError("Couldn't identify the OS architecture.") - else: - raise azclierror.BadRequestError(f"Unsuported OS architecture: {machine} is not currently supported") - - if os_architecture == "64bit": - sys_path = 'SysNative' if platform_architecture == '32bit' else 'System32' - else: - sys_path = 'System32' - - system_root = os.environ['SystemRoot'] - system32_path = os.path.join(system_root, sys_path) - ssh_path = os.path.join(system32_path, "openSSH", (ssh_command + ".exe")) - logger.debug("Platform architecture: %s", platform_architecture) - logger.debug("OS architecture: %s", os_architecture) - logger.debug("System Root: %s", system_root) - logger.debug("Attempting to run %s from path %s", ssh_command, ssh_path) - - if not os.path.isfile(ssh_path): - raise azclierror.UnclassifiedUserFault( - "Could not find " + ssh_command + ".exe on path " + ssh_path + ". " - "Make sure OpenSSH is installed correctly: " - "https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_install_firstuse . " - "Or use --ssh-client-folder to provide folder path with ssh executables. ") - return ssh_path @@ -302,27 +281,37 @@ def _start_cleanup(cert_file, private_key_file, public_key_file, delete_credenti def _terminate_cleanup(delete_keys, delete_cert, delete_credentials, cleanup_process, cert_file, private_key_file, public_key_file, log_file, connection_status): - - if delete_keys or delete_cert or delete_credentials: - if cleanup_process and cleanup_process.is_alive(): - cleanup_process.terminate() - # wait for process to terminate - t0 = time.time() - while cleanup_process.is_alive() and (time.time() - t0) < const.CLEANUP_AWAIT_TERMINATION_IN_SECONDS: - time.sleep(1) - - if log_file and os.path.isfile(log_file): - _print_error_messages_from_ssh_log(log_file, connection_status, delete_cert) - - # Make sure all files have been properly removed. - do_cleanup(delete_keys or delete_credentials, delete_cert or delete_credentials, - cert_file, private_key_file, public_key_file) - if log_file: - file_utils.delete_file(log_file, f"Couldn't delete temporary log file {log_file}. ", True) - if delete_keys: - # This is only true if keys were generated, so they must be in a temp folder. - temp_dir = os.path.dirname(cert_file) - file_utils.delete_folder(temp_dir, f"Couldn't delete temporary folder {temp_dir}", True) + try: + if connection_status and connection_status.stderr: + if connection_status.returncode != 0: + # Check if stderr is a proxy error + regex = ("{\"level\":\"fatal\",\"msg\":\"sshproxy: error copying information from the connection: " + ".*\",\"time\":\".*\"}.*") + if re.search(regex, connection_status.stderr): + logger.error("Please make sure SSH port is allowed using \"azcmagent config list\" in the target " + "Arc Server. Ensure SSHD is running on the target machine.") + print(connection_status.stderr) + finally: + if delete_keys or delete_cert or delete_credentials: + if cleanup_process and cleanup_process.is_alive(): + cleanup_process.terminate() + # wait for process to terminate + t0 = time.time() + while cleanup_process.is_alive() and (time.time() - t0) < const.CLEANUP_AWAIT_TERMINATION_IN_SECONDS: + time.sleep(1) + + if log_file and os.path.isfile(log_file): + _print_error_messages_from_ssh_log(log_file, connection_status, delete_cert) + + # Make sure all files have been properly removed. + do_cleanup(delete_keys or delete_credentials, delete_cert or delete_credentials, + cert_file, private_key_file, public_key_file) + if log_file: + file_utils.delete_file(log_file, f"Couldn't delete temporary log file {log_file}. ", True) + if delete_keys: + # This is only true if keys were generated, so they must be in a temp folder. + temp_dir = os.path.dirname(cert_file) + file_utils.delete_folder(temp_dir, f"Couldn't delete temporary folder {temp_dir}", True) def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, relay_info_path, ssh_client_folder): @@ -360,16 +349,3 @@ def _issue_config_cleanup_warning(delete_cert, delete_keys, is_arc, cert_file, r logger.warning("%s sensitive information%s. Please delete it once you no longer " "need this config file.", path_to_delete, items_to_delete) - - -def _get_connection_status(log_file, connection_status): - # pylint: disable=bare-except - if log_file and os.path.isfile(log_file): - try: - with open(log_file, 'r', encoding='utf-8') as ssh_client_log: - match = "debug1: Authentication succeeded" in ssh_client_log.read() - ssh_client_log.close() - except: - return False - return match and connection_status == 0 - return connection_status == 0 diff --git a/src/ssh/azext_ssh/tests/latest/test_ssh_info.py b/src/ssh/azext_ssh/tests/latest/test_ssh_info.py index 4523672a627..40a231d5088 100644 --- a/src/ssh/azext_ssh/tests/latest/test_ssh_info.py +++ b/src/ssh/azext_ssh/tests/latest/test_ssh_info.py @@ -121,17 +121,25 @@ def test_get_rg_and_vm_entry(self, mock_abspath): @mock.patch('os.path.abspath') def test_get_ip_entry(self, mock_abspath): - expected_lines = [ + expected_lines_aad = [ "Host ip", "\tUser user", "\tCertificateFile \"cert_path\"", "\tIdentityFile \"priv_path\"" ] + expected_lines_local_user = [ + "Host ip-user", + "\tHostName ip", + "\tUser user", + "\tCertificateFile \"cert_path\"", + "\tIdentityFile \"priv_path\"" + ] mock_abspath.side_effect = ["config_path", "pub_path", "priv_path", "cert_path", "client_path", "cred_folder"] session = ssh_info.ConfigSession("config", "rg", "vm", "ip", "pub", "priv", False, False, "user", "cert", None, "compute", "cred", None, "client/folder") - self.assertEqual(session._get_ip_entry(), expected_lines) + self.assertEqual(session._get_ip_entry(True), expected_lines_aad) + self.assertEqual(session._get_ip_entry(False), expected_lines_local_user) @mock.patch('os.path.abspath') def test_get_arc_entry(self, mock_abspath): @@ -185,7 +193,8 @@ def test_get_config_text_compute(self, mock_abspath): "\tCertificateFile \"cert_path\"", "\tIdentityFile \"priv_path\"", "\tPort port", - "Host ip", + "Host ip-user", + "\tHostName ip", "\tUser user", "\tCertificateFile \"cert_path\"", "\tIdentityFile \"priv_path\"", diff --git a/src/ssh/azext_ssh/tests/latest/test_ssh_utils.py b/src/ssh/azext_ssh/tests/latest/test_ssh_utils.py index 3bd0d3b6261..4d004a0b69d 100644 --- a/src/ssh/azext_ssh/tests/latest/test_ssh_utils.py +++ b/src/ssh/azext_ssh/tests/latest/test_ssh_utils.py @@ -16,7 +16,7 @@ class SSHUtilsTests(unittest.TestCase): @mock.patch.object(ssh_utils, '_start_cleanup') @mock.patch.object(ssh_utils, '_terminate_cleanup') @mock.patch.object(ssh_utils, '_get_ssh_client_path') - @mock.patch('subprocess.call') + @mock.patch('subprocess.run') @mock.patch('os.environ.copy') def test_start_ssh_connection_compute(self, mock_copy_env, mock_call, mock_path, mock_terminatecleanup, mock_startcleanup): @@ -37,13 +37,13 @@ def test_start_ssh_connection_compute(self, mock_copy_env, mock_call, mock_path, mock_path.assert_called_once_with('ssh', 'client') mock_startcleanup.assert_called_with('cert', 'priv', 'pub', False, True, True, ['arg1', 'arg2', 'arg3']) - mock_call.assert_called_once_with(expected_command, shell=platform.system() == 'Windows', env=expected_env) + mock_call.assert_called_once_with(expected_command, env=expected_env, stderr=mock.ANY, text=True) mock_terminatecleanup.assert_called_once_with(True, True, False, 'cleanup process', 'cert', 'priv', 'pub', 'log', 0) @mock.patch.object(ssh_utils, '_terminate_cleanup') @mock.patch('os.environ.copy') @mock.patch.object(ssh_utils, '_get_ssh_client_path') - @mock.patch('subprocess.call') + @mock.patch('subprocess.run') @mock.patch('azext_ssh.custom.connectivity_utils.format_relay_info_string') def test_start_ssh_connection_arc(self, mock_relay_str, mock_call, mock_path, mock_copy_env, mock_terminatecleanup): @@ -66,7 +66,7 @@ def test_start_ssh_connection_arc(self, mock_relay_str, mock_call, mock_path, mo mock_relay_str.assert_called_once_with('relay') mock_path.assert_called_once_with('ssh', 'client') - mock_call.assert_called_once_with(expected_command, shell=platform.system() == 'Windows', env=expected_env) + mock_call.assert_called_once_with(expected_command, env=expected_env, stderr=mock.ANY, text=True) mock_terminatecleanup.assert_called_once_with(False, False, False, None, 'cert', 'priv', 'pub', None, 0) @@ -139,93 +139,23 @@ def test_write_ssh_config_arc_overwrite(self, mock_create_file, mock_abspath, mo @mock.patch('os.path.join') @mock.patch('platform.system') @mock.patch('os.path.isfile') - def test_get_ssh_client_path_with_client_folder_non_windows(self, mock_isfile, mock_system, mock_join): - mock_join.return_value = "ssh_path" - mock_system.return_value = "Linux" - mock_isfile.return_value = True - actual_path = ssh_utils._get_ssh_client_path(ssh_client_folder='/client/folder') - self.assertEqual(actual_path, "ssh_path") - mock_join.assert_called_once_with('/client/folder', 'ssh') - mock_isfile.assert_called_once_with("ssh_path") - - @mock.patch('os.path.join') - @mock.patch('platform.system') - @mock.patch('os.path.isfile') - def test_get_ssh_client_path_with_client_folder_windows(self, mock_isfile, mock_system, mock_join): - mock_join.return_value = "ssh_keygen_path" - mock_system.return_value = "Windows" - mock_isfile.return_value = True - actual_path = ssh_utils._get_ssh_client_path(ssh_command='ssh-keygen', ssh_client_folder='/client/folder') - self.assertEqual(actual_path, "ssh_keygen_path.exe") - mock_join.assert_called_once_with('/client/folder', 'ssh-keygen') - mock_isfile.assert_called_once_with("ssh_keygen_path.exe") - - @mock.patch('os.path.join') - @mock.patch('platform.system') - @mock.patch('os.path.isfile') - def test_get_ssh_client_path_with_client_folder_no_file(self, mock_isfile, mock_system, mock_join): + def test_get_ssh_client_path_not_found(self, mock_isfile, mock_system, mock_join): mock_join.return_value = "ssh_path" mock_system.return_value = "Mac" mock_isfile.return_value = False - actual_path = ssh_utils._get_ssh_client_path(ssh_client_folder='/client/folder') - self.assertEqual(actual_path, "ssh") - mock_join.assert_called_once_with('/client/folder', 'ssh') - mock_isfile.assert_called_once_with("ssh_path") - - @mock.patch('platform.system') - def test_get_ssh_client_preinstalled_non_windows(self, mock_system): - mock_system.return_value = "Mac" - actual_path = ssh_utils._get_ssh_client_path() - self.assertEqual('ssh', actual_path) - mock_system.assert_called_once_with() - - def test_get_ssh_client_preinstalled_windows_32bit(self): - self._test_get_ssh_client_path_preinstalled_windows('32bit', 'x86', 'System32') - - def test_get_ssh_client_preinstalled_windows_64bitOS_32bitPlatform(self): - self._test_get_ssh_client_path_preinstalled_windows('32bit', 'x64', 'SysNative') + path = ssh_utils._get_ssh_client_path("ssh", "folder") + self.assertEqual(path, "ssh") - def test_get_ssh_client_preinstalled_windows_64bitOS_64bitPlatform(self): - self._test_get_ssh_client_path_preinstalled_windows('64bit', 'x64', 'System32') - - @mock.patch('platform.system') - @mock.patch('platform.architecture') - @mock.patch('platform.machine') @mock.patch('os.path.join') - @mock.patch('os.environ') + @mock.patch('platform.system') @mock.patch('os.path.isfile') - def _test_get_ssh_client_path_preinstalled_windows(self, platform_arch, os_arch, expected_sysfolder, mock_isfile, mock_environ, mock_join, mock_machine, mock_arch, mock_system): + def test_get_ssh_client_path_found(self, mock_isfile, mock_system, mock_join): + mock_join.return_value = "ssh_path" mock_system.return_value = "Windows" - mock_arch.return_value = (platform_arch, "foo", "bar") - mock_machine.return_value = os_arch - mock_environ.__getitem__.return_value = "rootpath" - mock_join.side_effect = ["system32path", "sshfilepath"] mock_isfile.return_value = True - - expected_join_calls = [ - mock.call("rootpath", expected_sysfolder), - mock.call("system32path", "openSSH", "ssh.exe") - ] - - actual_path = ssh_utils._get_ssh_client_path() - - self.assertEqual("sshfilepath", actual_path) - mock_system.assert_called_once_with() - mock_arch.assert_called_once_with() - mock_environ.__getitem__.assert_called_once_with("SystemRoot") - mock_join.assert_has_calls(expected_join_calls) - mock_isfile.assert_called_once_with("sshfilepath") - - @mock.patch('platform.system') - @mock.patch('platform.architecture') - @mock.patch('platform.machine') - @mock.patch('os.environ') - @mock.patch('os.path.isfile') - def test_get_ssh_path_windows_ssh_preinstalled_not_found(self, mock_isfile, mock_environ, mock_machine, mock_arch, mock_sys): - mock_sys.return_value = "Windows" - mock_arch.return_value = ("32bit", "foo", "bar") - mock_machine.return_value = "x64" - mock_environ.__getitem__.return_value = "rootpath" - mock_isfile.return_value = False - - self.assertRaises(azclierror.UnclassifiedUserFault, ssh_utils._get_ssh_client_path) + path = ssh_utils._get_ssh_client_path("ssh-keygen", "folder") + self.assertEqual(path, "ssh_path.exe") + + def test_get_ssh_client_preinstalled(self): + path = ssh_utils._get_ssh_client_path("ssh-keygen", None) + self.assertEqual(path, "ssh-keygen") \ No newline at end of file