Merge branch 'develop' into feature/apiddubny_bug_356_arm_template_fa…

…ils_during_installing_es_and_qx_scripts

package/cloudshell/cp/azure/domain/vm_management/operations/autoload_operation.py

@@ -1,3 +1,4 @@
+import netaddr
 from msrest.exceptions import AuthenticationError
 from msrestazure.azure_exceptions import CloudError
 
@@ -18,6 +19,15 @@ def __init__(self, vm_service, network_service):
         self.vm_service = vm_service
         self.network_service = network_service
 
+    def _validate_region(self, region):
+        """Verify Azure region
+
+        :param str region: Azure region
+        :return:
+        """
+        if not region:
+            raise AutoloadException("Region attribute can not be empty")
+
     def _validate_api_credentials(self, cloud_provider_model, logger):
         """Verify Azure API Credentials and return AzureClientsManager instance
 
@@ -115,13 +125,38 @@ def _register_azure_providers(self, resource_client, logger):
             logger.info("Register subscription with a {} resource provider".format(provider))
             resource_client.providers.register(provider)
 
-    def _validate_networks_in_use(self, sandbox_vnet, networks_in_use):
+    def _validate_cidr_format(self, cidr, logger):
+        """Validate that CIDR have a correct format. Example "10.10.10.10/24"
+
+        :param str cidr:
+        :param logging.Logger logger:
+        :return: True/False whether CIDR is valid or not
+        :rtype: bool
+        """
+        try:
+            netaddr.IPNetwork(cidr)
+        except netaddr.AddrFormatError:
+            logger.info("CIDR {} is in invalid format", exc_info=1)
+            return False
+        if '/' not in cidr:
+            return False
+
+        return True
+
+    def _validate_networks_in_use(self, sandbox_vnet, networks_in_use, logger):
         """Verify "Networks In Use" attribute
 
         :param sandbox_vnet: azure.mgmt.network.models.virtual_network.VirtualNetwork instance
         :param networks_in_use: list of used networks ["10.10.10.10/24", "20.20.20.20/24", ...]
+        :param logging.Logger logger:
         :return:
         """
+        for cidr in networks_in_use:
+            valid = self._validate_cidr_format(cidr, logger)
+            if not valid:
+                raise AutoloadException('CIDR {} under the "Networks In Use" attribute is not '
+                                        'in the valid format'.format(cidr))
+
         sandbox_cidrs = set([subnet.address_prefix for subnet in sandbox_vnet.subnets])
         cidrs = sandbox_cidrs - set(networks_in_use)
 
@@ -131,6 +166,19 @@ def _validate_networks_in_use(self, sandbox_vnet, networks_in_use):
 
             raise AutoloadException(error_msg)
 
+    def _validate_additional_mgmt_networks(self, additional_mgmt_networks, logger):
+        """Verify "Additional Mgmt Networks" attribute
+
+        :param additional_mgmt_networks: list of additional MGMT networks ["10.10.10.10/24", "20.20.20.20/24", ...]
+        :param logging.Logger logger:
+        :return:
+        """
+        for cidr in additional_mgmt_networks:
+            valid = self._validate_cidr_format(cidr, logger)
+            if not valid:
+                raise AutoloadException('CIDR {} under the "Additional Mgmt Networks" attribute is not '
+                                        'in the valid format'.format(cidr))
+
     def get_inventory(self, cloud_provider_model, logger):
         """Check that all needed resources are valid and present on the Azure
 
@@ -142,6 +190,8 @@ def get_inventory(self, cloud_provider_model, logger):
 
         azure_clients = self._validate_api_credentials(cloud_provider_model=cloud_provider_model, logger=logger)
 
+        self._validate_region(cloud_provider_model.region)
+
         self._register_azure_providers(resource_client=azure_clients.resource_client, logger=logger)
 
         self._validate_mgmt_resource_group(resource_client=azure_clients.resource_client,
@@ -174,7 +224,11 @@ def get_inventory(self, cloud_provider_model, logger):
                                    vm_size=cloud_provider_model.vm_size)
 
         self._validate_networks_in_use(sandbox_vnet=sandbox_vnet,
-                                       networks_in_use=cloud_provider_model.networks_in_use)
+                                       networks_in_use=cloud_provider_model.networks_in_use,
+                                       logger=logger)
+
+        self._validate_additional_mgmt_networks(additional_mgmt_networks=cloud_provider_model.additional_mgmt_networks,
+                                                logger=logger)
 
         logger.info("Autoload Operation was successfully completed")
 

package/requirements.txt

@@ -12,4 +12,5 @@ enum==0.4.6
 retrying==1.3.3
 pycrypto==2.6.1
 msrestazure>=0.4.0,<0.5.0
-httplib2==0.9.2
+httplib2==0.9.2
+netaddr==0.7.19

package/tests/test_cp/test_azure/test_domain/test_vm_management/test_operations/test_autoload_operation.py

@@ -30,6 +30,7 @@ def test_get_inventory(self, autoload_details_class):
         self.autoload_operation._validate_vnet = mock.MagicMock()
         self.autoload_operation._validate_vm_size = mock.MagicMock()
         self.autoload_operation._validate_networks_in_use = mock.MagicMock()
+        self.autoload_operation._validate_additional_mgmt_networks = mock.MagicMock()
 
         # Act
         result = self.autoload_operation.get_inventory(cloud_provider_model=cloud_provider_model,
@@ -42,6 +43,7 @@ def test_get_inventory(self, autoload_details_class):
         self.autoload_operation._validate_vnet.assert_called()
         self.autoload_operation._validate_vm_size.assert_called_once()
         self.autoload_operation._validate_networks_in_use.assert_called_once()
+        self.autoload_operation._validate_additional_mgmt_networks.assert_called_once()
 
     @mock.patch("cloudshell.cp.azure.domain.vm_management.operations.autoload_operation.AzureClientsManager")
     def test_validate_api_credentials(self, azure_clients_manager_class):
@@ -56,6 +58,15 @@ def test_validate_api_credentials(self, azure_clients_manager_class):
         # Verify
         self.assertEqual(ex.exception.message, "Failed to connect to Azure API, please check the log for more details")
 
+    def test_validate_region(self):
+        """Check that method will raise AutoloadException if region is empty"""
+        # Act
+        with self.assertRaises(AutoloadException) as ex:
+            self.autoload_operation._validate_region(region="")
+
+        # Verify
+        self.assertEqual(ex.exception.message, "Region attribute can not be empty")
+
     def test_validate_mgmt_resource_group_not_found(self):
         """Check that method will raise AutoloadException if management resource group doesn't exist on Azure"""
         resource_client = mock.MagicMock()
@@ -138,20 +149,77 @@ def test_register_azure_providers(self):
             mock.call("Microsoft.Network"),
             mock.call("Microsoft.Compute")])
 
-    @mock.patch("cloudshell.cp.azure.domain.vm_management.operations.autoload_operation.AzureClientsManager")
-    def test_validate_networks_in_use(self, azure_clients_manager_class):
+    def test_validate_networks_in_use_not_all_sandbox_subnets_listed(self):
         """Check that method will raise AutoloadException if "Networks In Use" attribute is invalid
 
         Verify that all subnets in the "sandbox" vNet are listed in the attribute"""
+        self.autoload_operation._validate_cidr_format = mock.MagicMock(return_value=True)
         networks_in_use = ["network2", "network4"]
         sandbox_vnet = mock.MagicMock(subnets=[mock.MagicMock(address_prefix="network1"),
                                                mock.MagicMock(address_prefix="network2")])
 
         # Act
         with self.assertRaises(AutoloadException) as ex:
             self.autoload_operation._validate_networks_in_use(sandbox_vnet=sandbox_vnet,
-                                                              networks_in_use=networks_in_use)
+                                                              networks_in_use=networks_in_use,
+                                                              logger=self.logger)
         # Verify
         self.assertEqual(ex.exception.message, 'The following subnets "network1" were found under the "{}" VNet '
                                                'in Azure and should be set in the "Network In Use" field.'
                          .format(sandbox_vnet.name))
+
+        self.autoload_operation._validate_cidr_format.assert_any_call("network2", self.logger)
+        self.autoload_operation._validate_cidr_format.assert_any_call("network4", self.logger)
+
+    def test_validate_networks_in_use_with_invalid_cidr_format(self):
+        """Check that method will raise AutoloadException if "Networks In Use" attribute have invalid CIDR format"""
+        self.autoload_operation._validate_cidr_format = mock.MagicMock(return_value=False)
+        networks_in_use = ["network2", "network4"]
+        sandbox_vnet = mock.MagicMock()
+
+        # Act
+        with self.assertRaises(AutoloadException) as ex:
+            self.autoload_operation._validate_networks_in_use(sandbox_vnet=sandbox_vnet,
+                                                              networks_in_use=networks_in_use,
+                                                              logger=self.logger)
+        # Verify
+        self.assertEqual(ex.exception.message, 'CIDR network2 under the "Networks In Use" attribute '
+                                               'is not in the valid format')
+
+    def test_validate_cidr_format(self):
+        """Check that method will return True is CIDR format is valid"""
+        cidr = "10.10.10.10/24"
+
+        result = self.autoload_operation._validate_cidr_format(cidr=cidr, logger=self.logger)
+
+        self.assertTrue(result)
+
+    def test_validate_cidr_format_invalid(self):
+        """Check that method will return False if given CIDR format is valid"""
+        cidr = "invalid CIDR"
+
+        result = self.autoload_operation._validate_cidr_format(cidr=cidr, logger=self.logger)
+
+        self.assertFalse(result)
+
+    def test_validate_cidr_format_single_ip(self):
+        """Check that method will return False if given CIDR is a single IP address"""
+        cidr = "10.10.10.10"
+
+        result = self.autoload_operation._validate_cidr_format(cidr=cidr, logger=self.logger)
+
+        self.assertFalse(result)
+
+    def test_validate_additional_mgmt_networks(self):
+        """Check that method will raise AutoloadException if "Additional Mgmt Networks" attr have invalid CIDR format"""
+        self.autoload_operation._validate_cidr_format = mock.MagicMock(return_value=False)
+        additional_mgmt_networks = ["network2", "network4"]
+
+        # Act
+        with self.assertRaises(AutoloadException) as ex:
+            self.autoload_operation._validate_additional_mgmt_networks(
+                additional_mgmt_networks=additional_mgmt_networks,
+                logger=self.logger)
+        # Verify
+        self.assertEqual(ex.exception.message, 'CIDR network2 under the "Additional Mgmt Networks" attribute '
+                                               'is not in the valid format')