From 5079fd9d5f1a97607edf5d3bdd3d33a2d3355c40 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Tue, 20 Feb 2024 16:55:46 -0500 Subject: [PATCH 01/10] update proxy version --- src/ssh/azext_ssh/connectivity_utils.py | 4 +++- src/ssh/azext_ssh/constants.py | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ssh/azext_ssh/connectivity_utils.py b/src/ssh/azext_ssh/connectivity_utils.py index 5242d48f488..2fbaf64be6c 100644 --- a/src/ssh/azext_ssh/connectivity_utils.py +++ b/src/ssh/azext_ssh/connectivity_utils.py @@ -235,7 +235,9 @@ def _get_proxy_filename_and_url(arc_proxy_folder): logger.debug("Platform OS: %s", operating_system) logger.debug("Platform architecture: %s", machine) - if machine.endswith('64') and 'ARM' not in machine.upper(): + if "arm64" in machine.lower() or "aarch64" in machine.lower(): + architecture = 'arm64' + elif machine.endswith('64'): architecture = 'amd64' elif machine.endswith('86'): architecture = '386' diff --git a/src/ssh/azext_ssh/constants.py b/src/ssh/azext_ssh/constants.py index c0d1f032476..6333076e126 100644 --- a/src/ssh/azext_ssh/constants.py +++ b/src/ssh/azext_ssh/constants.py @@ -7,8 +7,8 @@ AGENT_MINIMUM_VERSION_MAJOR = 1 AGENT_MINIMUM_VERSION_MINOR = 31 -CLIENT_PROXY_VERSION = "1.3.022941" -CLIENT_PROXY_RELEASE = "release21-04-23" +CLIENT_PROXY_VERSION = "1.3.026031" +CLIENT_PROXY_RELEASE = "release17-02-24" CLIENT_PROXY_STORAGE_URL = "https://sshproxysa.blob.core.windows.net" CLEANUP_TOTAL_TIME_LIMIT_IN_SECONDS = 120 CLEANUP_TIME_INTERVAL_IN_SECONDS = 10 From dbfb251e60357b22e9dbb90b67b9a5774b9efa5e Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Wed, 21 Feb 2024 14:31:02 -0500 Subject: [PATCH 02/10] download license --- src/ssh/azext_ssh/connectivity_utils.py | 37 +++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/ssh/azext_ssh/connectivity_utils.py b/src/ssh/azext_ssh/connectivity_utils.py index 2fbaf64be6c..81491851956 100644 --- a/src/ssh/azext_ssh/connectivity_utils.py +++ b/src/ssh/azext_ssh/connectivity_utils.py @@ -224,6 +224,8 @@ def get_client_side_proxy(arc_proxy_folder): os.chmod(install_location, os.stat(install_location).st_mode | stat.S_IXUSR) print_styled_text((Style.SUCCESS, f"SSH Client Proxy saved to {install_location}")) + _download_proxy_license() + return install_location @@ -270,6 +272,41 @@ def _get_proxy_filename_and_url(arc_proxy_folder): return request_uri, install_location, older_location +def _download_proxy_license(): + license_uri = f"{consts.CLIENT_PROXY_STORAGE_URL}/{consts.CLIENT_PROXY_RELEASE}/LICENSE.txt" + license_install_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", "LICENSE.txt"))) + + third_party_notices_uri = f"{consts.CLIENT_PROXY_STORAGE_URL}/{consts.CLIENT_PROXY_RELEASE}/ThirdPartyNotice.txt" + third_party_notice_install_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", "ThirdPartyNotice.txt"))) + + try: + license_content = _download_from_uri(license_uri) + except Exception as e: + logger.warning("Failed to download proxy license file from %s. Error: %s", license_uri, str(e)) + + try: + third_party_notice_content = _download_from_uri(third_party_notices_uri) + except Exception as e: + logger.warning("Failed to download proxy third party notice file from %s. Error: %s", third_party_notices_uri, str(e)) + + file_utils.write_to_file(license_install_location, 'wb', license_content, "Failed to create proxy license file. ") + file_utils.write_to_file(third_party_notice_install_location, 'wb', third_party_notice_content, "Failed to create proxy third party notice file. ") + + print_styled_text((Style.SUCCESS, f"SSH Client Proxy License and Third Party Notice saved to {license_install_location} and {third_party_notice_install_location}")) + + +def _download_from_uri(request_uri): + response_content = None + with urllib.request.urlopen(request_uri) as response: + response_content = response.read() + response.close() + + if response_content is None: + raise azclierror.ClientRequestError(f"Failed to download file from {request_uri}") + + return response_content + + def format_relay_info_string(relay_info): relay_info_string = json.dumps( { From 29edeac86f605f0d5691d5d2435695b5ac82b328 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Wed, 21 Feb 2024 16:49:18 -0500 Subject: [PATCH 03/10] Fix style --- src/ssh/azext_ssh/connectivity_utils.py | 37 +++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/ssh/azext_ssh/connectivity_utils.py b/src/ssh/azext_ssh/connectivity_utils.py index 81491851956..594650db112 100644 --- a/src/ssh/azext_ssh/connectivity_utils.py +++ b/src/ssh/azext_ssh/connectivity_utils.py @@ -273,26 +273,29 @@ def _get_proxy_filename_and_url(arc_proxy_folder): def _download_proxy_license(): + proxy_dir = os.path.join('~', ".clientsshproxy") license_uri = f"{consts.CLIENT_PROXY_STORAGE_URL}/{consts.CLIENT_PROXY_RELEASE}/LICENSE.txt" - license_install_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", "LICENSE.txt"))) + license_install_location = os.path.expanduser(os.path.join(proxy_dir, "LICENSE.txt")) - third_party_notices_uri = f"{consts.CLIENT_PROXY_STORAGE_URL}/{consts.CLIENT_PROXY_RELEASE}/ThirdPartyNotice.txt" - third_party_notice_install_location = os.path.expanduser(os.path.join('~', os.path.join(".clientsshproxy", "ThirdPartyNotice.txt"))) + notice_uri = f"{consts.CLIENT_PROXY_STORAGE_URL}/{consts.CLIENT_PROXY_RELEASE}/ThirdPartyNotice.txt" + notice_install_location = os.path.expanduser(os.path.join(proxy_dir, "ThirdPartyNotice.txt")) - try: - license_content = _download_from_uri(license_uri) - except Exception as e: - logger.warning("Failed to download proxy license file from %s. Error: %s", license_uri, str(e)) + _get_and_write_proxy_license_files(license_uri, license_install_location, "License") + _get_and_write_proxy_license_files(notice_uri, notice_install_location, "Third Party Notice") + +def _get_and_write_proxy_license_files(uri, install_location, target_name): try: - third_party_notice_content = _download_from_uri(third_party_notices_uri) - except Exception as e: - logger.warning("Failed to download proxy third party notice file from %s. Error: %s", third_party_notices_uri, str(e)) - - file_utils.write_to_file(license_install_location, 'wb', license_content, "Failed to create proxy license file. ") - file_utils.write_to_file(third_party_notice_install_location, 'wb', third_party_notice_content, "Failed to create proxy third party notice file. ") + license_content = _download_from_uri(uri) + file_utils.write_to_file(file_path=install_location, + mode='wb', + content=license_content, + error_message=f"Failed to create {target_name} file at {install_location}.") + # pylint: disable=broad-except + except Exception: + logger.warning("Failed to download Connection Proxy %s file from %s.", target_name, uri) - print_styled_text((Style.SUCCESS, f"SSH Client Proxy License and Third Party Notice saved to {license_install_location} and {third_party_notice_install_location}")) + print_styled_text((Style.SUCCESS, f"SSH Connection Proxy {target_name} saved to {install_location}.")) def _download_from_uri(request_uri): @@ -302,10 +305,10 @@ def _download_from_uri(request_uri): response.close() if response_content is None: - raise azclierror.ClientRequestError(f"Failed to download file from {request_uri}") - + raise azclierror.ClientRequestError(f"Failed to download file from {request_uri}") + return response_content - + def format_relay_info_string(relay_info): relay_info_string = json.dumps( From 95cebf6e6a606cf8a47e1c88470b73886a597583 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Fri, 1 Mar 2024 13:12:40 -0500 Subject: [PATCH 04/10] Style fixes --- src/ssh/HISTORY.md | 4 ++ src/ssh/azext_ssh/_process_helper.py | 1 + src/ssh/azext_ssh/rdp_utils.py | 1 + src/ssh/azext_ssh/ssh_utils.py | 2 +- src/ssh/azext_ssh/tests/latest/test_custom.py | 43 +++++++++---------- .../azext_ssh/tests/latest/test_rdp_utils.py | 13 +++--- .../azext_ssh/tests/latest/test_ssh_utils.py | 22 ++++------ .../tests/latest/test_target_os_utils.py | 10 +---- 8 files changed, 44 insertions(+), 52 deletions(-) diff --git a/src/ssh/HISTORY.md b/src/ssh/HISTORY.md index 0bc9ae91bd3..82028bcb86d 100644 --- a/src/ssh/HISTORY.md +++ b/src/ssh/HISTORY.md @@ -1,5 +1,9 @@ Release History =============== +2.0.3 +----- +* Add support to ARM64 clients when connecting to Arc Machines. Connect proxy now available for ARM64 architecture. + 2.0.2 ----- * [Bug Fix] Fix logic that checks for the OS of the target machine to avoid "cannot unpack non-iterable NoneType object" error diff --git a/src/ssh/azext_ssh/_process_helper.py b/src/ssh/azext_ssh/_process_helper.py index e8741d05216..11efba70af4 100644 --- a/src/ssh/azext_ssh/_process_helper.py +++ b/src/ssh/azext_ssh/_process_helper.py @@ -5,6 +5,7 @@ # pylint: disable=too-few-public-methods # pylint: disable=consider-using-with +# pylint: disable=superfluous-parens import subprocess from ctypes import WinDLL, c_int, c_size_t, Structure, WinError, sizeof, pointer diff --git a/src/ssh/azext_ssh/rdp_utils.py b/src/ssh/azext_ssh/rdp_utils.py index 76e45f4babf..016437aad6b 100644 --- a/src/ssh/azext_ssh/rdp_utils.py +++ b/src/ssh/azext_ssh/rdp_utils.py @@ -30,6 +30,7 @@ def start_rdp_connection(ssh_info, delete_keys, delete_cert): ssh_success = False retry_attempt = 0 retry_attempts_allowed = 0 + ssh_connection_t0 = time.time() resource_port = 3389 local_port = _get_open_port() diff --git a/src/ssh/azext_ssh/ssh_utils.py b/src/ssh/azext_ssh/ssh_utils.py index f3d1140d67d..7e69097e0b5 100644 --- a/src/ssh/azext_ssh/ssh_utils.py +++ b/src/ssh/azext_ssh/ssh_utils.py @@ -270,7 +270,7 @@ def _check_for_known_errors(error_message, delete_cert, log_lines): def check_for_service_config_delay_error(error_message): service_config_delay_error = False - regex = ("{\"level\":\"fatal\",\"msg\":\"sshproxy: error connecting to the address: 404 Endpoint does not exist.*") + regex = "{\"level\":\"fatal\",\"msg\":\"sshproxy: error connecting to the address: 404 Endpoint does not exist.*" if re.search(regex, error_message): service_config_delay_error = True return service_config_delay_error diff --git a/src/ssh/azext_ssh/tests/latest/test_custom.py b/src/ssh/azext_ssh/tests/latest/test_custom.py index 3f1951c5bfb..b9567e37e7e 100644 --- a/src/ssh/azext_ssh/tests/latest/test_custom.py +++ b/src/ssh/azext_ssh/tests/latest/test_custom.py @@ -19,7 +19,7 @@ class SshCustomCommandTest(unittest.TestCase): @mock.patch('azext_ssh.custom._assert_args') @mock.patch('azext_ssh.ssh_info.SSHSession') @mock.patch('azext_ssh.resource_type_utils.decide_resource_type') - def test_ssh_vm(self, mock_type, mock_info, mock_assert, mock_do_op): + def test_ssh_vm(self, mock_type, mock_info, mock_assert, mock_do_op): cmd = mock.Mock() ssh_info = mock.Mock() mock_info.return_value = ssh_info @@ -32,20 +32,20 @@ def test_ssh_vm(self, mock_type, mock_info, mock_assert, mock_do_op): mock_assert.assert_called_once_with("rg", "vm", "ip", "type", "cert", "username") mock_type.assert_called_once_with(cmd, ssh_info) mock_do_op.assert_called_once_with(cmd, ssh_info, ssh_utils.start_ssh_connection) - + @mock.patch('azext_ssh.custom._do_ssh_op') @mock.patch('azext_ssh.custom._assert_args') @mock.patch('azext_ssh.ssh_info.SSHSession') @mock.patch('azext_ssh.resource_type_utils.decide_resource_type') @mock.patch('platform.system') - def test_ssh_vm_rdp(self, mock_sys, mock_type, mock_info, mock_assert, mock_do_op): + def test_ssh_vm_rdp(self, mock_sys, mock_type, mock_info, mock_assert, mock_do_op): cmd = mock.Mock() ssh_info = mock.Mock() mock_info.return_value = ssh_info mock_sys.return_value = 'Windows' cmd.cli_ctx.data = {'safe_params': []} - + custom.ssh_vm(cmd, "rg", "vm", "ip", "public", "private", False, "username", "cert", "port", "ssh_folder", False, "type", "proxy", True, False, ['-vvv']) mock_info.assert_called_once_with("rg", "vm", "ip", "public", "private", False, "username", "cert", "port", "ssh_folder", ['-vvv'], False, "type", "proxy", None, True, False) @@ -57,13 +57,13 @@ def test_ssh_vm_rdp(self, mock_sys, mock_type, mock_info, mock_assert, mock_do_o @mock.patch('azext_ssh.custom._assert_args') @mock.patch('azext_ssh.ssh_info.SSHSession') @mock.patch('azext_ssh.resource_type_utils.decide_resource_type') - def test_ssh_vm_debug(self, mock_type, mock_info, mock_assert, mock_do_op): + def test_ssh_vm_debug(self, mock_type, mock_info, mock_assert, mock_do_op): cmd = mock.Mock() ssh_info = mock.Mock() mock_info.return_value = ssh_info cmd.cli_ctx.data = {'safe_params': ['--debug']} - + custom.ssh_vm(cmd, "rg", "vm", "ip", "public", "private", False, "username", "cert", "port", "ssh_folder", False, "type", "proxy", False, False, []) mock_info.assert_called_once_with("rg", "vm", "ip", "public", "private", False, "username", "cert", "port", "ssh_folder", ['-vvv'], False, "type", "proxy", None, False, False) @@ -89,7 +89,7 @@ def test_ssh_vm_delete_credentials_cloudshell(self, mock_info, mock_assert, mock mock_assert.assert_called_once_with("rg", "vm", "ip", "type", "cert", "username") mock_type.assert_called_once_with(cmd, ssh_info) mock_op.assert_called_once_with(cmd, ssh_info, ssh_utils.start_ssh_connection) - + @mock.patch('os.environ.get') def test_delete_credentials_not_cloudshell(self, mock_getenv): mock_getenv.return_value = None @@ -106,7 +106,7 @@ def test_delete_credentials_not_cloudshell(self, mock_getenv): @mock.patch('os.path.join') def test_ssh_config_no_cred_folder(self, mock_join, mock_info, mock_isdir, mock_dirname, mock_type, mock_do_op, mock_assert): cmd = mock.Mock() - + config_info = mock.Mock() config_info.ip = "ip" config_info.resource_group_name = "rg" @@ -130,7 +130,7 @@ def test_ssh_config_no_cred_folder(self, mock_join, mock_info, mock_isdir, mock_ mock_type.assert_called_once_with(cmd, config_info) mock_assert.assert_called_once_with("rg", "vm", "ip", "type", "cert", "user") mock_do_op.assert_called_once_with(cmd, config_info, ssh_utils.write_ssh_config) - + @mock.patch('azext_ssh.custom._assert_args') @mock.patch('azext_ssh.custom._do_ssh_op') @mock.patch('azext_ssh.resource_type_utils.decide_resource_type') @@ -145,13 +145,12 @@ def test_ssh_config_cred_folder(self, mock_dirname, mock_isdir, mock_info, mock_ mock_dirname.return_value = "config_folder" custom.ssh_config(cmd, "config", "rg", "vm", "ip", None, None, True, False, "user", "cert", "port", "type", "cred", "proxy", "client", False) - + mock_type.assert_called_once_with(cmd, config_info) mock_info.assert_called_once_with("config", "rg", "vm", "ip", None, None, True, False, "user", "cert", "port", "type", "cred", "proxy", "client", False) mock_assert.assert_called_once_with("rg", "vm", "ip", "type", "cert", "user") mock_op.assert_called_once_with(cmd, config_info, ssh_utils.write_ssh_config) - def test_ssh_config_credentials_folder_and_key(self): cmd = mock.Mock() self.assertRaises( @@ -169,7 +168,7 @@ def test_ssh_cert_no_args(self): cmd = mock.Mock() self.assertRaises( azclierror.RequiredArgumentMissingError, custom.ssh_cert, cmd) - + @mock.patch('os.path.isdir') def test_ssh_cert_cert_file_missing(self, mock_isdir): cmd = mock.Mock() @@ -187,7 +186,7 @@ def test_ssh_cert(self, mock_write_cert, mock_get_keys, mock_abspath, mock_isdir mock_abspath.side_effect = ['/pubkey/path', '/cert/path', '/client/path'] mock_get_keys.return_value = "pubkey", "privkey", False mock_write_cert.return_value = "cert", "username" - + custom.ssh_cert(cmd, "cert", "pubkey", "ssh/folder") mock_get_keys.assert_called_once_with('/pubkey/path', None, None, '/client/path') @@ -195,7 +194,7 @@ def test_ssh_cert(self, mock_write_cert, mock_get_keys, mock_abspath, mock_isdir def test_assert_args_invalid_resource_type(self): self.assertRaises(azclierror.InvalidArgumentValueError, custom._assert_args, 'rg', 'vm', 'ip', "Microsoft.Network", 'cert', 'user') - + def test_assert_args_no_ip_or_vm(self): self.assertRaises(azclierror.RequiredArgumentMissingError, custom._assert_args, None, None, None, None, None, None) @@ -207,7 +206,7 @@ def test_assert_args_ip_with_vm_or_rg(self): self.assertRaises(azclierror.MutuallyExclusiveArgumentError, custom._assert_args, None, "vm", "ip", None, None, None) self.assertRaises(azclierror.MutuallyExclusiveArgumentError, custom._assert_args, "rg", None, "ip", None, None, None) self.assertRaises(azclierror.MutuallyExclusiveArgumentError, custom._assert_args, "rg", "vm", "ip", None, None, None) - + def test_assert_args_cert_with_no_user(self): self.assertRaises(azclierror.MutuallyExclusiveArgumentError, custom._assert_args, None, None, "ip", None, "certificate", None) @@ -247,7 +246,7 @@ def test_check_or_create_public_private_files_defaults(self, mock_join, mock_isf @mock.patch('os.path.isdir') @mock.patch('os.path.isfile') @mock.patch('os.path.join') - def test_check_or_create_public_private_files_defaults_with_cred_folder(self,mock_join, mock_isfile, mock_isdir, mock_create): + def test_check_or_create_public_private_files_defaults_with_cred_folder(self, mock_join, mock_isfile, mock_isdir, mock_create): mock_isfile.return_value = True mock_isdir.return_value = True mock_join.side_effect = ['/cred/folder/id_rsa.pub', '/cred/folder/id_rsa'] @@ -266,7 +265,7 @@ def test_check_or_create_public_private_files_defaults_with_cred_folder(self,moc mock_create.assert_has_calls([ mock.call('/cred/folder/id_rsa', '/ssh/client') ]) - + @mock.patch('os.path.isfile') def test_check_or_create_public_private_files_no_public(self, mock_isfile): mock_isfile.side_effect = [False] @@ -284,7 +283,6 @@ def test_check_or_create_public_private_files_no_public(self, mock_isfile): self.assertEqual(delete, False) mock_isfile.assert_has_calls([mock.call("key.pub"), mock.call('key')]) - @mock.patch('os.path.isfile') @mock.patch('os.path.join') def test_check_or_create_public_private_files_no_private(self, mock_join, mock_isfile): @@ -298,7 +296,6 @@ def test_check_or_create_public_private_files_no_private(self, mock_join, mock_i mock.call("public"), mock.call("private") ]) - @mock.patch('builtins.open') @mock.patch('oschmod.set_mode') @@ -345,7 +342,7 @@ def test_get_modulus_exponent_parse_error(self, mock_open, mock_isfile, mock_par mock_parser_obj.parse.side_effect = ValueError self.assertRaises(azclierror.FileOperationError, custom._get_modulus_exponent, 'file') - + @mock.patch('azext_ssh.ssh_utils.get_ssh_cert_principals') @mock.patch('os.path.join') @mock.patch('azext_ssh.custom._check_or_create_public_private_files') @@ -416,7 +413,7 @@ def test_do_ssh_op_no_public_ip(self, mock_ip, mock_check_files): mock_check_files.assert_not_called() mock_ip.assert_called_once_with(cmd, "rg", "vm", False) mock_op.assert_not_called() - + @mock.patch('azext_ssh.connectivity_utils.get_client_side_proxy') @mock.patch('azext_ssh.connectivity_utils.get_relay_information') @mock.patch('azext_ssh.ssh_utils.start_ssh_connection') @@ -440,7 +437,7 @@ def test_do_ssh_op_arc_local_user(self, mock_get_cert, mock_check_keys, mock_sta mock_op.assert_called_once_with(op_info, False, False) mock_get_cert.assert_not_called() mock_check_keys.assert_not_called() - + @mock.patch('azext_ssh.connectivity_utils.get_client_side_proxy') @mock.patch('azext_ssh.custom.connectivity_utils.get_relay_information') @mock.patch('azext_ssh.ssh_utils.get_ssh_cert_principals') @@ -489,6 +486,6 @@ def test_do_ssh_arc_op_aad_user(self, mock_cert_exp, mock_start_ssh, mock_write_ mock_get_proxy.assert_called_once_with('proxy') mock_get_relay_info.assert_called_once_with(cmd, 'rg', 'vm', 'Microsoft.HybridCompute/machines', 3600, 'port', False) mock_op.assert_called_once_with(op_info, False, True) - + if __name__ == '__main__': unittest.main() diff --git a/src/ssh/azext_ssh/tests/latest/test_rdp_utils.py b/src/ssh/azext_ssh/tests/latest/test_rdp_utils.py index 15252bb9ef8..9d8d11dccf2 100644 --- a/src/ssh/azext_ssh/tests/latest/test_rdp_utils.py +++ b/src/ssh/azext_ssh/tests/latest/test_rdp_utils.py @@ -10,6 +10,7 @@ from azext_ssh import ssh_info from azext_ssh import ssh_utils + class RDPUtilsTest(unittest.TestCase): @mock.patch('os.environ.copy') @mock.patch.object(ssh_utils, 'get_ssh_client_path') @@ -24,13 +25,13 @@ def test_start_ssh_tunnel(self, mock_popen, mock_relay, mock_path, mock_env): op_info.proxy_path = "proxy" op_info.relay_info = "relay" - mock_env.return_value = {'var1':'value1', 'var2':'value2', 'var3':'value3'} + mock_env.return_value = {'var1': 'value1', 'var2': 'value2', 'var3': 'value3'} mock_path.return_value = 'ssh' mock_relay.return_value = 'relay_string' mock_popen.return_value = 'ssh_process' expected_command = ['ssh', "vm", '-l', 'user', '-o', 'ProxyCommand=\"proxy\" -p port', '-i', 'priv', '-o', 'CertificateFile=\"cert\"', 'arg1', 'arg2', '-v'] - expected_env = {'var1':'value1', 'var2':'value2', 'var3':'value3', 'SSHPROXY_RELAY_INFO':'relay_string'} + expected_env = {'var1': 'value1', 'var2': 'value2', 'var3': 'value3', 'SSHPROXY_RELAY_INFO': 'relay_string'} ssh_sub, print_logs = rdp_utils.start_ssh_tunnel(op_info) @@ -62,7 +63,7 @@ def test_get_rdp_path(self, mock_isfile, mock_join, mock_env, mock_arch, mock_sy mock_join.assert_has_calls(expected_join_calls) mock_isfile.assert_called_once_with('root/sys/rdp') - #start rdp connection + # start rdp connection @mock.patch.object(rdp_utils, '_get_open_port') @mock.patch.object(rdp_utils, 'is_local_port_open') @mock.patch.object(rdp_utils, 'start_ssh_tunnel') @@ -81,17 +82,17 @@ def test_start_rdp_connection(self, mock_terminate, mock_rdp, mock_wait, mock_tu mock_getport.return_value = 1020 mock_isopen.return_value = True ssh_pro = mock.Mock() - #ssh_pro.return_value.poll.return_value = None + # ssh_pro.return_value.poll.return_value = None mock_tunnel.return_value = ssh_pro, False mock_wait.return_value = True, [], False rdp_utils.start_rdp_connection(op_info, True, True) mock_terminate.assert_called_once_with(ssh_pro, [], False) - #mock_rdp.assert_called_once_with(1020) + # mock_rdp.assert_called_once_with(1020) mock_tunnel.assert_called_once_with(op_info) mock_wait.assert_called_once_with(ssh_pro, False) if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main() 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 5d2c7958018..0b08c9289cb 100644 --- a/src/ssh/azext_ssh/tests/latest/test_ssh_utils.py +++ b/src/ssh/azext_ssh/tests/latest/test_ssh_utils.py @@ -32,9 +32,9 @@ def test_start_ssh_connection_compute_aad_windows(self, mock_system, mock_copy_e mock_call.return_value = 0 mock_path.return_value = 'ssh' mock_call.return_value = ssh_process - mock_copy_env.return_value = {'var1':'value1', 'var2':'value2', 'var3':'value3'} + mock_copy_env.return_value = {'var1': 'value1', 'var2': 'value2', 'var3': 'value3'} expected_command = ['ssh', 'ip', '-l', 'user', '-i', 'priv', '-o', 'CertificateFile=\"cert\"', '-p', 'port', '-v', 'arg1', 'arg2', 'arg3'] - expected_env = {'var1':'value1', 'var2':'value2', 'var3':'value3'} + expected_env = {'var1': 'value1', 'var2': 'value2', 'var3': 'value3'} ssh_utils.start_ssh_connection(op_info, True, True) @@ -64,9 +64,9 @@ def test_start_ssh_connection_compute_local_linux(self, mock_system, mock_copy_e mock_call.return_value = 0 mock_path.return_value = 'ssh' mock_call.return_value = ssh_process - mock_copy_env.return_value = {'var1':'value1', 'var2':'value2', 'var3':'value3'} + mock_copy_env.return_value = {'var1': 'value1', 'var2': 'value2', 'var3': 'value3'} expected_command = ['ssh', 'ip', '-l', 'user', '-i', 'priv', '-o', 'CertificateFile=\"cert\"', '-p', 'port', 'arg1', 'arg2', 'arg3'] - expected_env = {'var1':'value1', 'var2':'value2', 'var3':'value3'} + expected_env = {'var1': 'value1', 'var2': 'value2', 'var3': 'value3'} ssh_utils.start_ssh_connection(op_info, False, False) @@ -75,7 +75,6 @@ def test_start_ssh_connection_compute_local_linux(self, mock_system, mock_copy_e mock_wait.assert_called_once_with(ssh_process, op_info, False, False) mock_cleanup.assert_called_once_with(False, False, False, 'cert', 'priv', 'pub') - @mock.patch.object(ssh_utils, 'do_cleanup') @mock.patch.object(ssh_utils, '_check_ssh_logs_for_common_errors') @mock.patch.object(ssh_utils, 'get_ssh_client_path') @@ -99,10 +98,10 @@ def test_start_ssh_connection_arc_aad_windows(self, mock_platform, mock_relay_st mock_platform.return_value = 'Windows' mock_call.return_value = ssh_process mock_relay_str.return_value = 'relay_string' - mock_copy_env.return_value = {'var1':'value1', 'var2':'value2', 'var3':'value3'} + mock_copy_env.return_value = {'var1': 'value1', 'var2': 'value2', 'var3': 'value3'} mock_path.return_value = 'ssh' expected_command = ['ssh', 'vm', '-l', 'user', '-o', 'ProxyCommand=\"proxy\" -p port', '-i', 'priv', '-o', 'CertificateFile=\"cert\"', '-v', 'arg1'] - expected_env = {'var1':'value1', 'var2':'value2', 'var3':'value3', 'SSHPROXY_RELAY_INFO':'relay_string'} + expected_env = {'var1': 'value1', 'var2': 'value2', 'var3': 'value3', 'SSHPROXY_RELAY_INFO': 'relay_string'} ssh_utils.start_ssh_connection(op_info, True, True) @@ -112,7 +111,6 @@ def test_start_ssh_connection_arc_aad_windows(self, mock_platform, mock_relay_st mock_cleanup.assert_called_once_with(True, True, False, 'cert', 'priv', 'pub') mock_read.assert_called_once_with(ssh_process, op_info, True, True) - @mock.patch.object(ssh_utils, 'do_cleanup') @mock.patch.object(ssh_utils, '_wait_to_delete_credentials') @mock.patch.object(ssh_utils, 'get_ssh_client_path') @@ -121,7 +119,6 @@ def test_start_ssh_connection_arc_aad_windows(self, mock_platform, mock_relay_st @mock.patch('azext_ssh.custom.connectivity_utils.format_relay_info_string') @mock.patch('platform.system') def test_start_ssh_connection_arc_local_linux(self, mock_platform, mock_relay_str, mock_call, mock_copy_env, mock_path, mock_wait, mock_cleanup): - op_info = ssh_info.SSHSession("rg", "vm", None, None, None, False, "user", None, "port", None, ['arg1'], False, "Microsoft.HybridCompute/machines", None, None, False, False) op_info.public_key_file = "pub" op_info.private_key_file = "priv" @@ -136,10 +133,10 @@ def test_start_ssh_connection_arc_local_linux(self, mock_platform, mock_relay_st mock_platform.return_value = 'Linux' mock_call.return_value = ssh_process mock_relay_str.return_value = 'relay_string' - mock_copy_env.return_value = {'var1':'value1', 'var2':'value2', 'var3':'value3'} + mock_copy_env.return_value = {'var1': 'value1', 'var2': 'value2', 'var3': 'value3'} mock_path.return_value = 'ssh' expected_command = ['ssh', 'vm', '-l', 'user', '-o', 'ProxyCommand=\"proxy\" -p port', '-i', 'priv', '-o', 'CertificateFile=\"cert\"', 'arg1'] - expected_env = {'var1':'value1', 'var2':'value2', 'var3':'value3', 'SSHPROXY_RELAY_INFO':'relay_string'} + expected_env = {'var1': 'value1', 'var2': 'value2', 'var3': 'value3', 'SSHPROXY_RELAY_INFO': 'relay_string'} ssh_utils.start_ssh_connection(op_info, False, False) @@ -149,7 +146,6 @@ def test_start_ssh_connection_arc_local_linux(self, mock_platform, mock_relay_st mock_cleanup.assert_called_once_with(False, False, False, 'cert', 'priv', 'pub') mock_wait.assert_called_once_with(ssh_process, op_info, False, False) - @mock.patch.object(ssh_utils, '_issue_config_cleanup_warning') @mock.patch('os.path.abspath') def test_write_ssh_config_ip_and_vm_compute_append(self, mock_abspath, mock_warning): @@ -215,7 +211,7 @@ def test_write_ssh_config_arc_overwrite(self, mock_create_file, mock_abspath, mo mock_warning.assert_called_once_with(True, True, True, "cert", "relay", "client") mock_open.assert_called_once_with("config", 'w', encoding='utf-8') mock_file.write.assert_called_once_with('\n'.join(expected_lines)) - + @mock.patch('os.path.join') @mock.patch('platform.system') @mock.patch('os.path.isfile') diff --git a/src/ssh/azext_ssh/tests/latest/test_target_os_utils.py b/src/ssh/azext_ssh/tests/latest/test_target_os_utils.py index 03a4a80b5f0..6e48cd5852a 100644 --- a/src/ssh/azext_ssh/tests/latest/test_target_os_utils.py +++ b/src/ssh/azext_ssh/tests/latest/test_target_os_utils.py @@ -29,9 +29,7 @@ def test_get_arc_os(self, mock_get_arc): self.assertEqual(os, "os_type") self.assertEqual(agent, "arc_agent_version") - - @mock.patch('azext_ssh.aaz.latest.hybrid_compute.machine.Show', autospec=True) def test_get_arc_os_exception(self, mock_get_arc): cmd = mock.Mock() @@ -44,7 +42,6 @@ def test_get_arc_os_exception(self, mock_get_arc): self.assertEqual(os, None) self.assertEqual(agent, None) - @mock.patch('azext_ssh.aaz.latest.connected_v_mwarev_sphere.virtual_machine.Show') def test_get_vmware_os(self, mock_get_vmware): cmd = mock.Mock() @@ -68,9 +65,7 @@ def test_get_vmware_os(self, mock_get_vmware): self.assertEqual(os, "os_type") self.assertEqual(agent, "agent_version") - - @mock.patch('azext_ssh.aaz.latest.connected_v_mwarev_sphere.virtual_machine.Show', autospec=True) def test_get_vmware_os_exception(self, mock_get_vmware): cmd = mock.Mock() @@ -84,8 +79,5 @@ def test_get_vmware_os_exception(self, mock_get_vmware): self.assertEqual(agent, None) - - - if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main() From 02cafb241e5e977259de5da75ebe3d37653321ed Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Fri, 1 Mar 2024 13:50:16 -0500 Subject: [PATCH 05/10] Style fixes --- src/ssh/azext_ssh/tests/latest/test_custom.py | 16 +++++++-------- .../azext_ssh/tests/latest/test_rdp_utils.py | 2 +- .../tests/latest/test_resource_type_utils.py | 20 +++++++++---------- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/ssh/azext_ssh/tests/latest/test_custom.py b/src/ssh/azext_ssh/tests/latest/test_custom.py index b9567e37e7e..b8130983058 100644 --- a/src/ssh/azext_ssh/tests/latest/test_custom.py +++ b/src/ssh/azext_ssh/tests/latest/test_custom.py @@ -25,7 +25,7 @@ def test_ssh_vm(self, mock_type, mock_info, mock_assert, mock_do_op): mock_info.return_value = ssh_info cmd.cli_ctx.data = {'safe_params': []} - + custom.ssh_vm(cmd, "rg", "vm", "ip", "public", "private", False, "username", "cert", "port", "ssh_folder", False, "type", "proxy", False, False, ['-vvv']) mock_info.assert_called_once_with("rg", "vm", "ip", "public", "private", False, "username", "cert", "port", "ssh_folder", ['-vvv'], False, "type", "proxy", None, False, False) @@ -308,7 +308,7 @@ def test_write_cert_file(self, mock_mode, mock_open): mock_mode.assert_called_once_with("publickey-aadcert.pub", 0o644) mock_open.assert_called_once_with("publickey-aadcert.pub", 'w', encoding='utf-8') mock_file.write.assert_called_once_with("ssh-rsa-cert-v01@openssh.com cert") - + @mock.patch('azext_ssh.rsa_parser.RSAParser') @mock.patch('os.path.isfile') @mock.patch('builtins.open') @@ -370,9 +370,9 @@ def test_do_ssh_op_aad_user_compute(self, mock_write_cert, mock_ssh_creds, mock_ profile._adal_cache = True profile.get_msal_token.return_value = "username", "certificate" mock_join.return_value = "public-aadcert.pub" - + custom._do_ssh_op(cmd, op_info, mock_op) - + mock_check_files.assert_called_once_with("publicfile", "privatefile", None, "/client/folder") mock_ip.assert_not_called() mock_get_mod_exp.assert_called_once_with("public") @@ -390,7 +390,7 @@ def test_do_ssh_op_local_user_compute(self, mock_ip, mock_check_files): op_info.public_key_file = "publicfile" op_info.private_key_file = "privatefile" op_info.cert_file = "cert" - op_info.ssh_client_folder = "/client/folder" + op_info.ssh_client_folder = "/client/folder" custom._do_ssh_op(cmd, op_info, mock_op) @@ -421,7 +421,7 @@ def test_do_ssh_op_no_public_ip(self, mock_ip, mock_check_files): @mock.patch('azext_ssh.custom._get_and_write_certificate') def test_do_ssh_op_arc_local_user(self, mock_get_cert, mock_check_keys, mock_start_ssh, mock_get_relay_info, mock_get_proxy): mock_get_relay_info.return_value = ('relay', False) - cmd = mock.Mock() + cmd = mock.Mock() mock_op = mock.Mock() op_info = ssh_info.SSHSession("rg", "vm", None, None, None, False, "user", None, "port", None, [], False, "Microsoft.HybridCompute/machines", None, None, False, False) @@ -431,7 +431,7 @@ def test_do_ssh_op_arc_local_user(self, mock_get_cert, mock_check_keys, mock_sta op_info.ssh_proxy_folder = "proxy" custom._do_ssh_op(cmd, op_info, mock_op) - + mock_get_proxy.assert_called_once_with('proxy') mock_get_relay_info.assert_called_once_with(cmd, 'rg', 'vm', 'Microsoft.HybridCompute/machines', None, "port", False) mock_op.assert_called_once_with(op_info, False, False) @@ -448,7 +448,7 @@ def test_do_ssh_op_arc_local_user(self, mock_get_cert, mock_check_keys, mock_sta @mock.patch('azext_ssh.custom._write_cert_file') @mock.patch('azext_ssh.ssh_utils.start_ssh_connection') @mock.patch('azext_ssh.ssh_utils.get_certificate_lifetime') - def test_do_ssh_arc_op_aad_user(self, mock_cert_exp, mock_start_ssh, mock_write_cert, mock_ssh_creds, mock_get_mod_exp, mock_check_files, + def test_do_ssh_arc_op_aad_user(self, mock_cert_exp, mock_start_ssh, mock_write_cert, mock_ssh_creds, mock_get_mod_exp, mock_check_files, mock_join, mock_principal, mock_get_relay_info, mock_get_proxy): mock_get_proxy.return_value = '/path/to/proxy' diff --git a/src/ssh/azext_ssh/tests/latest/test_rdp_utils.py b/src/ssh/azext_ssh/tests/latest/test_rdp_utils.py index 9d8d11dccf2..2f90618d4cc 100644 --- a/src/ssh/azext_ssh/tests/latest/test_rdp_utils.py +++ b/src/ssh/azext_ssh/tests/latest/test_rdp_utils.py @@ -87,7 +87,7 @@ def test_start_rdp_connection(self, mock_terminate, mock_rdp, mock_wait, mock_tu mock_wait.return_value = True, [], False rdp_utils.start_rdp_connection(op_info, True, True) - + mock_terminate.assert_called_once_with(ssh_pro, [], False) # mock_rdp.assert_called_once_with(1020) mock_tunnel.assert_called_once_with(op_info) diff --git a/src/ssh/azext_ssh/tests/latest/test_resource_type_utils.py b/src/ssh/azext_ssh/tests/latest/test_resource_type_utils.py index e04cfa762d5..8648238c254 100644 --- a/src/ssh/azext_ssh/tests/latest/test_resource_type_utils.py +++ b/src/ssh/azext_ssh/tests/latest/test_resource_type_utils.py @@ -12,8 +12,9 @@ from azure.cli.core import azclierror + class SshResourceTypeUtilsCommandTest(unittest.TestCase): - + @mock.patch('azext_ssh.resource_type_utils._list_types_of_resources_with_provided_name') def test_decide_resource_type_ip(self, mock_list_types): cmd = mock.Mock() @@ -21,22 +22,21 @@ def test_decide_resource_type_ip(self, mock_list_types): self.assertEqual(resource_type_utils.decide_resource_type(cmd, op_info), "Microsoft.Compute/virtualMachines") mock_list_types.assert_not_called() - ################################ Test Resource Type Provided ############################## - + # Test Resource Type Provided @mock.patch('azext_ssh.resource_type_utils._list_types_of_resources_with_provided_name') def test_decide_resource_type_resourcetype_arc(self, mock_list_types): cmd = mock.Mock() mock_list_types.return_value = {'microsoft.hybridcompute/machines', 'microsoft.compute/virtualmachines'} op_info = ssh_info.SSHSession("rg", "vm", None, None, None, False, None, None, None, None, [], False, "Microsoft.HybridCompute/machines", None, None, False, False) self.assertEqual(resource_type_utils.decide_resource_type(cmd, op_info), "Microsoft.HybridCompute/machines") - + @mock.patch('azext_ssh.resource_type_utils._list_types_of_resources_with_provided_name') def test_decide_resource_type_resourcetype_compute(self, mock_list_types): cmd = mock.Mock() mock_list_types.return_value = {'microsoft.hybridcompute/machines', 'microsoft.compute/virtualmachines', 'microsoft.connectedvmwarevsphere/virtualmachines'} op_info = ssh_info.SSHSession("rg", "vm", None, None, None, False, None, None, None, None, [], False, "Microsoft.Compute/virtualMachines", None, None, False, False) self.assertEqual(resource_type_utils.decide_resource_type(cmd, op_info), "Microsoft.Compute/virtualMachines") - + @mock.patch('azext_ssh.resource_type_utils._list_types_of_resources_with_provided_name') def test_decide_resource_type_resourcetype_vmware(self, mock_list_types): cmd = mock.Mock() @@ -44,22 +44,21 @@ def test_decide_resource_type_resourcetype_vmware(self, mock_list_types): op_info = ssh_info.SSHSession("rg", "vm", None, None, None, False, None, None, None, None, [], False, "Microsoft.connectedvmwarevsphere/virtualMachines", None, None, False, False) self.assertEqual(resource_type_utils.decide_resource_type(cmd, op_info), "Microsoft.ConnectedVMwarevSphere/virtualMachines") - - ############################ Test Legacy Resource Type (Resource Provider) ########################## + # Test Legacy Resource Type (Resource Provider) @mock.patch('azext_ssh.resource_type_utils._list_types_of_resources_with_provided_name') def test_decide_resource_type_resourcetype_arc_legacy(self, mock_list_types): cmd = mock.Mock() mock_list_types.return_value = {'microsoft.hybridcompute/machines', 'microsoft.compute/virtualmachines'} op_info = ssh_info.SSHSession("rg", "vm", None, None, None, False, None, None, None, None, [], False, "Microsoft.HybridCompute", None, None, False, False) self.assertEqual(resource_type_utils.decide_resource_type(cmd, op_info), "Microsoft.HybridCompute/machines") - + @mock.patch('azext_ssh.resource_type_utils._list_types_of_resources_with_provided_name') def test_decide_resource_type_resourcetype_compute_legacy(self, mock_list_types): cmd = mock.Mock() mock_list_types.return_value = {'microsoft.hybridcompute/machines', 'microsoft.compute/virtualmachines', 'microsoft.connectedvmwarevsphere/virtualmachines'} op_info = ssh_info.SSHSession("rg", "vm", None, None, None, False, None, None, None, None, [], False, "Microsoft.Compute", None, None, False, False) self.assertEqual(resource_type_utils.decide_resource_type(cmd, op_info), "Microsoft.Compute/virtualMachines") - + @mock.patch('azext_ssh.resource_type_utils._list_types_of_resources_with_provided_name') def test_decide_resource_type_resourcetype_vmware_legacy(self, mock_list_types): cmd = mock.Mock() @@ -67,7 +66,7 @@ def test_decide_resource_type_resourcetype_vmware_legacy(self, mock_list_types): op_info = ssh_info.SSHSession("rg", "vm", None, None, None, False, None, None, None, None, [], False, "Microsoft.connectedvmwarevsphere", None, None, False, False) self.assertEqual(resource_type_utils.decide_resource_type(cmd, op_info), "Microsoft.ConnectedVMwarevSphere/virtualMachines") - ############################# Test No Resource Type Provided ################################### + # Test No Resource Type Provided @mock.patch('azext_ssh.resource_type_utils._list_types_of_resources_with_provided_name') def test_decide_resource_type_more_than_one(self, mock_list_types): cmd = mock.Mock() @@ -105,6 +104,5 @@ def test_decide_resource_type_rg_vmware(self, mock_list_types): op_info = ssh_info.SSHSession("rg", "vm", None, None, None, False, None, None, None, None, [], False, None, None, None, False, False) self.assertEqual(resource_type_utils.decide_resource_type(cmd, op_info), "Microsoft.ConnectedVMwarevSphere/virtualMachines") - if __name__ == '__main__': unittest.main() \ No newline at end of file From 3b6eeddd8c66668f91deb4a2f6d6c8d95c7ad8dd Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Fri, 1 Mar 2024 14:49:52 -0500 Subject: [PATCH 06/10] Style fixes --- src/ssh/azext_ssh/rdp_utils.py | 2 +- .../tests/latest/test_resource_type_utils.py | 2 +- src/ssh/azext_ssh/tests/latest/test_ssh_info.py | 3 +-- src/ssh/azext_ssh/tests/latest/test_ssh_utils.py | 13 ++++++------- .../tests/latest/test_target_os_utils.py | 15 ++++++++------- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/ssh/azext_ssh/rdp_utils.py b/src/ssh/azext_ssh/rdp_utils.py index 016437aad6b..2c21b267ebc 100644 --- a/src/ssh/azext_ssh/rdp_utils.py +++ b/src/ssh/azext_ssh/rdp_utils.py @@ -30,7 +30,6 @@ def start_rdp_connection(ssh_info, delete_keys, delete_cert): ssh_success = False retry_attempt = 0 retry_attempts_allowed = 0 - ssh_connection_t0 = time.time() resource_port = 3389 local_port = _get_open_port() @@ -65,6 +64,7 @@ def start_rdp_connection(ssh_info, delete_keys, delete_cert): call_rdp(local_port) finally: + # pylint: disable=used-before-assignment if ssh_success: ssh_connection_data = {'Context.Default.AzureCLI.SSHConnectionDurationInMinutes': (time.time() - ssh_connection_t0) / 60} diff --git a/src/ssh/azext_ssh/tests/latest/test_resource_type_utils.py b/src/ssh/azext_ssh/tests/latest/test_resource_type_utils.py index 8648238c254..09ad972d312 100644 --- a/src/ssh/azext_ssh/tests/latest/test_resource_type_utils.py +++ b/src/ssh/azext_ssh/tests/latest/test_resource_type_utils.py @@ -105,4 +105,4 @@ def test_decide_resource_type_rg_vmware(self, mock_list_types): self.assertEqual(resource_type_utils.decide_resource_type(cmd, op_info), "Microsoft.ConnectedVMwarevSphere/virtualMachines") if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main() 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 54ff59ed16d..0254722d0d7 100644 --- a/src/ssh/azext_ssh/tests/latest/test_ssh_info.py +++ b/src/ssh/azext_ssh/tests/latest/test_ssh_info.py @@ -43,7 +43,6 @@ def test_ssh_session(self, mock_abspath): self.assertEqual(session.winrdp, True) self.assertEqual(session.yes_without_prompt, False) - def test_ssh_session_get_host(self): session = ssh_info.SSHSession(None, None, "ip", None, None, False, "user", None, None, None, [], False, "Microsoft.Compute/virtualMachines", None, None, False, False) self.assertEqual("ip", session.get_host()) @@ -121,7 +120,6 @@ def test_get_rg_and_vm_entry(self, mock_abspath): self.assertEqual(session._get_rg_and_vm_entry(True), expected_lines_aad) self.assertEqual(session._get_rg_and_vm_entry(False), expected_lines_local_user) - @mock.patch('os.path.abspath') def test_get_ip_entry(self, mock_abspath): expected_lines_aad = [ @@ -240,5 +238,6 @@ def test_get_config_text_arc(self, create_file, mock_abspath): self.assertEqual(session.get_config_text(True), expected_lines_aad) self.assertEqual(session.get_config_text(False), expected_lines_local_user) + if __name__ == '__main__': unittest.main() 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 0b08c9289cb..f00784fa8c6 100644 --- a/src/ssh/azext_ssh/tests/latest/test_ssh_utils.py +++ b/src/ssh/azext_ssh/tests/latest/test_ssh_utils.py @@ -10,7 +10,7 @@ from azext_ssh import ssh_info -class SSHUtilsTests(unittest.TestCase): +class SSHUtilsTests(unittest.TestCase): @mock.patch.object(ssh_utils, 'do_cleanup') @mock.patch.object(ssh_utils, '_check_ssh_logs_for_common_errors') @mock.patch.object(ssh_utils, 'get_ssh_client_path') @@ -42,7 +42,7 @@ def test_start_ssh_connection_compute_aad_windows(self, mock_system, mock_copy_e mock_call.assert_called_once_with(expected_command, stderr=mock.ANY, env=expected_env, encoding='utf-8') mock_read.assert_called_once_with(ssh_process, op_info, True, True) mock_cleanup.assert_called_once_with(True, True, False, 'cert', 'priv', 'pub') - + @mock.patch.object(ssh_utils, 'do_cleanup') @mock.patch.object(ssh_utils, '_wait_to_delete_credentials') @mock.patch.object(ssh_utils, 'get_ssh_client_path') @@ -83,7 +83,7 @@ def test_start_ssh_connection_compute_local_linux(self, mock_system, mock_copy_e @mock.patch('azext_ssh.custom.connectivity_utils.format_relay_info_string') @mock.patch('platform.system') def test_start_ssh_connection_arc_aad_windows(self, mock_platform, mock_relay_str, mock_call, mock_copy_env, mock_path, mock_read, mock_cleanup): - + op_info = ssh_info.SSHSession("rg", "vm", None, None, None, False, "user", None, "port", None, ['arg1'], False, "Microsoft.HybridCompute/machines", None, None, False, False) op_info.public_key_file = "pub" op_info.private_key_file = "priv" @@ -94,7 +94,7 @@ def test_start_ssh_connection_arc_aad_windows(self, mock_platform, mock_relay_st ssh_process = mock.Mock() ssh_process.poll.return_value = 0 - + mock_platform.return_value = 'Windows' mock_call.return_value = ssh_process mock_relay_str.return_value = 'relay_string' @@ -129,7 +129,7 @@ def test_start_ssh_connection_arc_local_linux(self, mock_platform, mock_relay_st ssh_process = mock.Mock() ssh_process.poll.return_value = 0 - + mock_platform.return_value = 'Linux' mock_call.return_value = ssh_process mock_relay_str.return_value = 'relay_string' @@ -282,7 +282,7 @@ def _test_get_ssh_client_path_preinstalled_windows(self, platform_arch, os_arch, mock.call("rootpath", expected_sysfolder), mock.call("system32path", "openSSH", "ssh.exe") ] - + actual_path = ssh_utils.get_ssh_client_path() self.assertEqual("sshfilepath", actual_path) @@ -292,7 +292,6 @@ def _test_get_ssh_client_path_preinstalled_windows(self, platform_arch, os_arch, 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') diff --git a/src/ssh/azext_ssh/tests/latest/test_target_os_utils.py b/src/ssh/azext_ssh/tests/latest/test_target_os_utils.py index 6e48cd5852a..9c402655cd7 100644 --- a/src/ssh/azext_ssh/tests/latest/test_target_os_utils.py +++ b/src/ssh/azext_ssh/tests/latest/test_target_os_utils.py @@ -8,8 +8,9 @@ from azext_ssh import target_os_utils + class TargetOSUtilsTest(unittest.TestCase): - + @mock.patch('azext_ssh.aaz.latest.hybrid_compute.machine.Show') def test_get_arc_os(self, mock_get_arc): cmd = mock.Mock() @@ -22,7 +23,7 @@ def test_get_arc_os(self, mock_get_arc): "agentVersion": "arc_agent_version" } } - + mock_get_arc.return_value = showclass os, agent = target_os_utils._get_arc_server_os(cmd, "rg", "vm") @@ -36,7 +37,7 @@ def test_get_arc_os_exception(self, mock_get_arc): cmd.cli_ctx = mock.Mock() mock_get_arc.return_value.side_effect = mock.Mock(side_effect=Exception('Test')) - + os, agent = target_os_utils._get_arc_server_os(cmd, "rg", "vm") self.assertEqual(os, None) @@ -49,16 +50,16 @@ def test_get_vmware_os(self, mock_get_vmware): showclass = mock.Mock() showclass.return_value = { - "osProfile":{ + "osProfile": { "osType": "os_type" }, - "properties":{ + "properties": { "guestAgentProfile": { "agentVersion": "agent_version" } } } - + mock_get_vmware.return_value = showclass os, agent = target_os_utils._get_connected_vmware_os(cmd, "rg", "vm") @@ -72,7 +73,7 @@ def test_get_vmware_os_exception(self, mock_get_vmware): cmd.cli_ctx = mock.Mock() mock_get_vmware.return_value.side_effect = mock.Mock(side_effect=Exception('Test')) - + os, agent = target_os_utils._get_connected_vmware_os(cmd, "rg", "vm") self.assertEqual(os, None) From 46cd677976526b6e2ecc18c419712b45c2655640 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Fri, 1 Mar 2024 15:35:45 -0500 Subject: [PATCH 07/10] Style fixes --- src/ssh/azext_ssh/rdp_utils.py | 3 ++- src/ssh/azext_ssh/tests/latest/test_ssh_info.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ssh/azext_ssh/rdp_utils.py b/src/ssh/azext_ssh/rdp_utils.py index 2c21b267ebc..cab42670696 100644 --- a/src/ssh/azext_ssh/rdp_utils.py +++ b/src/ssh/azext_ssh/rdp_utils.py @@ -3,6 +3,8 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- +# pylint: disable=used-before-assignment + import os import platform import subprocess @@ -64,7 +66,6 @@ def start_rdp_connection(ssh_info, delete_keys, delete_cert): call_rdp(local_port) finally: - # pylint: disable=used-before-assignment if ssh_success: ssh_connection_data = {'Context.Default.AzureCLI.SSHConnectionDurationInMinutes': (time.time() - ssh_connection_t0) / 60} 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 0254722d0d7..9de4674e455 100644 --- a/src/ssh/azext_ssh/tests/latest/test_ssh_info.py +++ b/src/ssh/azext_ssh/tests/latest/test_ssh_info.py @@ -61,7 +61,7 @@ def test_ssh_session_build_args_hyvridcompute(self, mock_abspath): session = ssh_info.SSHSession("rg", "vm", "ip", "pub", "priv", False, "user", "cert", "port", "client/folder", [], None, "Microsoft.HybridCompute/machines", None, None, True, False) session.proxy_path = "proxy_path" self.assertEqual(["-o", "ProxyCommand=\"proxy_path\" -p port", "-i", "priv_path", "-o", "CertificateFile=\"cert_path\""], session.build_args()) - + @mock.patch('os.path.abspath') def test_config_session(self, mock_abspath): mock_abspath.side_effect = ["config_path", "pub_path", "priv_path", "cert_path", "client_path", "proxy_path", "cred_path"] From 5f5ed7be576524a4c6900ddac247677f1061b4b9 Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Fri, 1 Mar 2024 16:23:54 -0500 Subject: [PATCH 08/10] Run checks again --- src/ssh/azext_ssh/connectivity_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ssh/azext_ssh/connectivity_utils.py b/src/ssh/azext_ssh/connectivity_utils.py index 594650db112..13eb2011ff3 100644 --- a/src/ssh/azext_ssh/connectivity_utils.py +++ b/src/ssh/azext_ssh/connectivity_utils.py @@ -248,7 +248,7 @@ def _get_proxy_filename_and_url(arc_proxy_folder): else: raise azclierror.BadRequestError(f"Unsuported architecture: {machine} is not currently supported") - # define the request url and install location based on the os and architecture + # define the request url and install location based on the os and architecture. proxy_name = f"sshProxy_{operating_system.lower()}_{architecture}" request_uri = (f"{consts.CLIENT_PROXY_STORAGE_URL}/{consts.CLIENT_PROXY_RELEASE}" f"/{proxy_name}_{consts.CLIENT_PROXY_VERSION}") From 48f712763b0818f61977ce3d71f9e1159baf61cb Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut Date: Mon, 4 Mar 2024 08:14:34 -0800 Subject: [PATCH 09/10] Remove preview warning for RDP feature --- src/ssh/azext_ssh/custom.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ssh/azext_ssh/custom.py b/src/ssh/azext_ssh/custom.py index 34054ebd3fc..f13cd706596 100644 --- a/src/ssh/azext_ssh/custom.py +++ b/src/ssh/azext_ssh/custom.py @@ -53,7 +53,6 @@ def ssh_vm(cmd, resource_group_name=None, vm_name=None, ssh_ip=None, public_key_ if platform.system() != 'Windows': raise azclierror.BadRequestError("RDP connection is not supported for this platform. " "Supported platforms: Windows") - logger.warning("RDP feature is in preview.") op_call = rdp_utils.start_rdp_connection ssh_session = ssh_info.SSHSession(resource_group_name, vm_name, ssh_ip, public_key_file, From 7308cb5be3704e750f0fb6c666f4a69ce4e390da Mon Sep 17 00:00:00 2001 From: Vivian Thiebaut <81188381+vthiebaut10@users.noreply.github.com> Date: Tue, 5 Mar 2024 07:23:57 -0800 Subject: [PATCH 10/10] Update src/ssh/HISTORY.md Co-authored-by: Xing Zhou --- src/ssh/HISTORY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ssh/HISTORY.md b/src/ssh/HISTORY.md index 82028bcb86d..d3aa0470d99 100644 --- a/src/ssh/HISTORY.md +++ b/src/ssh/HISTORY.md @@ -1,6 +1,6 @@ Release History =============== -2.0.3 +upcoming ----- * Add support to ARM64 clients when connecting to Arc Machines. Connect proxy now available for ARM64 architecture.