From dd53367cbc2848abffb2028d383a7aeb6002e70c Mon Sep 17 00:00:00 2001 From: Harold Zeng Date: Wed, 11 Nov 2020 08:47:20 +0000 Subject: [PATCH 1/4] wrap filenotfound error --- .../azure/cli/command_modules/privatedns/custom.py | 7 ++++++- .../privatedns/tests/latest/test_privatedns_commands.py | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/privatedns/custom.py b/src/azure-cli/azure/cli/command_modules/privatedns/custom.py index bfa22bbc177..f63f444e238 100644 --- a/src/azure-cli/azure/cli/command_modules/privatedns/custom.py +++ b/src/azure-cli/azure/cli/command_modules/privatedns/custom.py @@ -33,7 +33,12 @@ def import_zone(cmd, resource_group_name, private_zone_name, file_name): import sys from azure.mgmt.privatedns.models import RecordSet - file_text = read_file_content(file_name) + from azure.cli.core.azclierror import FileOperationError + try: + file_text = read_file_content(file_name) + except FileNotFoundError as e: + raise FileOperationError(e) + zone_obj = parse_zone_file(file_text, private_zone_name) origin = private_zone_name record_sets = {} diff --git a/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py b/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py index 36fafb7c253..2cb7cfda239 100644 --- a/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py +++ b/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py @@ -955,6 +955,13 @@ def _test_PrivateDnsZone(self, zone_name, filename): # verify that each record in the original import is unchanged after export/re-import self._check_records(records1, records2) + @ResourceGroupPreparer(name_prefix='test_Private_Dns_import_file_not_found') + def test_Private_Dns_import_file_not_found(self, resource_group): + from azure.cli.core.azclierror import FileOperationError + with self.assertRaises(FileOperationError) as e: + self._test_PrivateDnsZone('404zone.com', 'non_existing_zone_description_file.txt') + self.assertEqual(e.errno, 1) + @ResourceGroupPreparer(name_prefix='cli_private_dns_zone1_import') def test_Private_Dns_Zone1_Import(self, resource_group): self._test_PrivateDnsZone('zone1.com', 'zone1.txt') From b864a2d05cec62f5061dc7b66f9f524396e0f0e9 Mon Sep 17 00:00:00 2001 From: Harold Zeng Date: Wed, 11 Nov 2020 10:08:06 +0000 Subject: [PATCH 2/4] add more catches --- .../azure/cli/command_modules/network/custom.py | 12 +++++++++--- .../network/tests/latest/test_dns_commands.py | 16 ++++++++++++++-- .../cli/command_modules/privatedns/custom.py | 12 +++++++++--- .../tests/latest/test_privatedns_commands.py | 16 ++++++++++++++-- 4 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/network/custom.py b/src/azure-cli/azure/cli/command_modules/network/custom.py index e290a326179..544dc95289e 100644 --- a/src/azure-cli/azure/cli/command_modules/network/custom.py +++ b/src/azure-cli/azure/cli/command_modules/network/custom.py @@ -2046,11 +2046,17 @@ def import_zone(cmd, resource_group_name, zone_name, file_name): logger.warning("In the future, zone name will be case insensitive.") RecordSet = cmd.get_models('RecordSet', resource_type=ResourceType.MGMT_NETWORK_DNS) - from azure.cli.core.azclierror import FileOperationError + from azure.cli.core.azclierror import FileOperationError, UnknownError try: file_text = read_file_content(file_name) - except FileNotFoundError as e: - raise FileOperationError(e) + except FileNotFoundError: + raise FileOperationError("No such file: " + str(file_name)) + except IsADirectoryError: + raise FileOperationError("Is a directory: " + str(file_name)) + except PermissionError: + raise FileOperationError("Permission denied: " + str(file_name)) + except OSError as e: + raise UnknownError(e) zone_obj = parse_zone_file(file_text, zone_name) diff --git a/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py b/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py index 5f57c925653..9f7aa666ee3 100644 --- a/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py +++ b/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py @@ -61,12 +61,24 @@ def _test_zone(self, zone_name, filename): @live_only() @ResourceGroupPreparer(name_prefix='test_dns_import_file_not_found') - def test_dns_import_file_not_found(self, resource_group): + def test_dns_import_file_operation_error(self, resource_group): + import sys + if sys.platform != 'linux': + self.skip('This test should run on Linux platform') + from azure.cli.core.azclierror import FileOperationError - with self.assertRaises(FileOperationError) as e: + with self.assertRaisesRegexp(FileOperationError, 'No such file: ') as e: self._test_zone('404zone.com', 'non_existing_zone_description_file.txt') self.assertEqual(e.errno, 1) + with self.assertRaisesRegexp(FileOperationError, 'Is a directory: ') as e: + self._test_zone('404zone.com', '') + self.assertEqual(e.errno, 1) + + with self.assertRaisesRegexp(FileOperationError, 'Permission denied: ') as e: + self._test_zone('404zone.com', '/root/') + self.assertEqual(e.errno, 1) + @ResourceGroupPreparer(name_prefix='cli_dns_zone1_import') def test_dns_zone1_import(self, resource_group): self._test_zone('zone1.com', 'zone1.txt') diff --git a/src/azure-cli/azure/cli/command_modules/privatedns/custom.py b/src/azure-cli/azure/cli/command_modules/privatedns/custom.py index f63f444e238..ec6b9cdafb1 100644 --- a/src/azure-cli/azure/cli/command_modules/privatedns/custom.py +++ b/src/azure-cli/azure/cli/command_modules/privatedns/custom.py @@ -33,11 +33,17 @@ def import_zone(cmd, resource_group_name, private_zone_name, file_name): import sys from azure.mgmt.privatedns.models import RecordSet - from azure.cli.core.azclierror import FileOperationError + from azure.cli.core.azclierror import FileOperationError, UnknownError try: file_text = read_file_content(file_name) - except FileNotFoundError as e: - raise FileOperationError(e) + except FileNotFoundError: + raise FileOperationError("No such file: " + str(file_name)) + except IsADirectoryError: + raise FileOperationError("Is a directory: " + str(file_name)) + except PermissionError: + raise FileOperationError("Permission denied: " + str(file_name)) + except OSError as e: + raise UnknownError(e) zone_obj = parse_zone_file(file_text, private_zone_name) origin = private_zone_name diff --git a/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py b/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py index 2cb7cfda239..132087a2109 100644 --- a/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py +++ b/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py @@ -956,12 +956,24 @@ def _test_PrivateDnsZone(self, zone_name, filename): self._check_records(records1, records2) @ResourceGroupPreparer(name_prefix='test_Private_Dns_import_file_not_found') - def test_Private_Dns_import_file_not_found(self, resource_group): + def test_Private_Dns_import_file_operation_error_linux(self, resource_group): + import sys + if sys.platform != 'linux': + self.skip('This test should run on Linux platform') + from azure.cli.core.azclierror import FileOperationError - with self.assertRaises(FileOperationError) as e: + with self.assertRaisesRegexp(FileOperationError, 'No such file: ') as e: self._test_PrivateDnsZone('404zone.com', 'non_existing_zone_description_file.txt') self.assertEqual(e.errno, 1) + with self.assertRaisesRegexp(FileOperationError, 'Is a directory: ') as e: + self._test_PrivateDnsZone('404zone.com', '') + self.assertEqual(e.errno, 1) + + with self.assertRaisesRegexp(FileOperationError, 'Permission denied: ') as e: + self._test_PrivateDnsZone('404zone.com', '/root/') + self.assertEqual(e.errno, 1) + @ResourceGroupPreparer(name_prefix='cli_private_dns_zone1_import') def test_Private_Dns_Zone1_Import(self, resource_group): self._test_PrivateDnsZone('zone1.com', 'zone1.txt') From bb88f4bc923b4cb42e5d8d92a2e08c42c29293bb Mon Sep 17 00:00:00 2001 From: Jianhui Harold Date: Wed, 11 Nov 2020 18:39:37 +0800 Subject: [PATCH 3/4] add tests on Windows --- .../network/tests/latest/test_dns_commands.py | 17 +++++++++++++++++ .../tests/latest/test_privatedns_commands.py | 16 ++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py b/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py index 9f7aa666ee3..f29540d050a 100644 --- a/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py +++ b/src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py @@ -79,6 +79,23 @@ def test_dns_import_file_operation_error(self, resource_group): self._test_zone('404zone.com', '/root/') self.assertEqual(e.errno, 1) + @live_only() + @ResourceGroupPreparer(name_prefix='test_dns_import_file_operation_error_windows') + def test_dns_import_file_operation_error_windows(self, resource_group): + import sys + if sys.platform != 'win32': + self.skip('This test should run on Windows platform') + + from azure.cli.core.azclierror import FileOperationError + with self.assertRaisesRegexp(FileOperationError, 'No such file: ') as e: + self._test_zone('404zone.com', 'non_existing_zone_description_file.txt') + self.assertEqual(e.errno, 1) + + # Difference with Linux platform while reading a directory + with self.assertRaisesRegexp(FileOperationError, 'Permission denied:') as e: + self._test_zone('404zone.com', '.') + self.assertEqual(e.errno, 1) + @ResourceGroupPreparer(name_prefix='cli_dns_zone1_import') def test_dns_zone1_import(self, resource_group): self._test_zone('zone1.com', 'zone1.txt') diff --git a/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py b/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py index 132087a2109..0c1f455932b 100644 --- a/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py +++ b/src/azure-cli/azure/cli/command_modules/privatedns/tests/latest/test_privatedns_commands.py @@ -974,6 +974,22 @@ def test_Private_Dns_import_file_operation_error_linux(self, resource_group): self._test_PrivateDnsZone('404zone.com', '/root/') self.assertEqual(e.errno, 1) + @ResourceGroupPreparer(name_prefix='test_dns_import_file_operation_error_windows') + def test_Private_Dns_import_file_operation_error_windows(self, resource_group): + import sys + if sys.platform != 'win32': + self.skip('This test should run on Windows platform') + + from azure.cli.core.azclierror import FileOperationError + with self.assertRaisesRegexp(FileOperationError, 'No such file: ') as e: + self._test_PrivateDnsZone('404zone.com', 'non_existing_zone_description_file.txt') + self.assertEqual(e.errno, 1) + + # Difference with Linux platform while reading a directory + with self.assertRaisesRegexp(FileOperationError, 'Permission denied:') as e: + self._test_PrivateDnsZone('404zone.com', '.') + self.assertEqual(e.errno, 1) + @ResourceGroupPreparer(name_prefix='cli_private_dns_zone1_import') def test_Private_Dns_Zone1_Import(self, resource_group): self._test_PrivateDnsZone('zone1.com', 'zone1.txt') From 31b1f4d6e53d139a05f6c73885abc55fbd78ab00 Mon Sep 17 00:00:00 2001 From: Harold Zeng Date: Thu, 12 Nov 2020 02:43:54 +0000 Subject: [PATCH 4/4] fix pylint error --- src/azure-cli/azure/cli/command_modules/privatedns/custom.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/privatedns/custom.py b/src/azure-cli/azure/cli/command_modules/privatedns/custom.py index ec6b9cdafb1..6861ec3f31b 100644 --- a/src/azure-cli/azure/cli/command_modules/privatedns/custom.py +++ b/src/azure-cli/azure/cli/command_modules/privatedns/custom.py @@ -27,7 +27,7 @@ def list_privatedns_zones(cmd, resource_group_name=None): return client.list() -# pylint: disable=too-many-statements, too-many-locals +# pylint: disable=too-many-statements, too-many-locals, too-many-branches def import_zone(cmd, resource_group_name, private_zone_name, file_name): from azure.cli.core.util import read_file_content import sys