server: Allow creating network with duplicate name (#3807)

Add a global setting to disable creating networks with same name in an account

Add a global setting to disable creating network without
mentioning the start and end IPv4 or IPv6 address

By default we can create networks with the same name in the account.
Sometimes we should not create the networks with same name.
This change adds a global setting which prevents creating the network with same name.
The default value is true and set it to false to prevent creating network with same names.

Also its possible to create a shared network without mentioning the
start and the end IPv4 or IPv6 address.
This change adds a global setting which prevents creating a shared
network without specifying the start and the end IPv4 or IPv6 address

.travis.yml

@@ -44,6 +44,7 @@ env:
              smoke/test_affinity_groups_projects
              smoke/test_async_job
              smoke/test_create_list_domain_account_project
+             smoke/test_create_network
              smoke/test_deploy_vgpu_enabled_vm
              smoke/test_deploy_vm_iso
              smoke/test_deploy_vm_root_resize

engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java

@@ -120,4 +120,6 @@ public interface NetworkDao extends GenericDao<NetworkVO, Long>, StateDao<State,
     int getNonSystemNetworkCountByVpcId(long vpcId);
 
     List<NetworkVO> listNetworkVO(List<Long> idset);
+
+    List<NetworkVO> listByAccountIdNetworkName(long accountId, String name);
 }

engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java

@@ -104,6 +104,7 @@ public class NetworkDaoImpl extends GenericDaoBase<NetworkVO, Long>implements Ne
     @PostConstruct
     protected void init() {
         AllFieldsSearch = createSearchBuilder();
+        AllFieldsSearch.and("name", AllFieldsSearch.entity().getName(), Op.EQ);
         AllFieldsSearch.and("trafficType", AllFieldsSearch.entity().getTrafficType(), Op.EQ);
         AllFieldsSearch.and("cidr", AllFieldsSearch.entity().getCidr(), Op.EQ);
         AllFieldsSearch.and("broadcastType", AllFieldsSearch.entity().getBroadcastDomainType(), Op.EQ);
@@ -697,4 +698,13 @@ public class NetworkDaoImpl extends GenericDaoBase<NetworkVO, Long>implements Ne
         sc_2.addAnd("removed", SearchCriteria.Op.EQ, null);
         return this.search(sc_2, searchFilter_2);
     }
+
+    @Override
+    public List<NetworkVO> listByAccountIdNetworkName(final long accountId, final String name) {
+        final SearchCriteria<NetworkVO> sc = AllFieldsSearch.create();
+        sc.setParameters("account", accountId);
+        sc.setParameters("name", name);
+
+        return listBy(sc, null);
+    }
 }

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

@@ -55,6 +55,8 @@ import org.apache.cloudstack.api.command.user.vm.ListNicsCmd;
 import org.apache.cloudstack.api.response.AcquirePodIpCmdResponse;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.framework.messagebus.MessageBus;
 import org.apache.cloudstack.framework.messagebus.PublishScope;
@@ -107,6 +109,7 @@ import com.cloud.network.dao.FirewallRulesDao;
 import com.cloud.network.dao.IPAddressDao;
 import com.cloud.network.dao.IPAddressVO;
 import com.cloud.network.dao.LoadBalancerDao;
+import com.cloud.network.dao.NetworkAccountDao;
 import com.cloud.network.dao.NetworkDao;
 import com.cloud.network.dao.NetworkDetailVO;
 import com.cloud.network.dao.NetworkDetailsDao;
@@ -200,9 +203,14 @@ import com.cloud.vm.dao.VMInstanceDao;
 /**
  * NetworkServiceImpl implements NetworkService.
  */
-public class NetworkServiceImpl extends ManagerBase implements NetworkService {
+public class NetworkServiceImpl extends ManagerBase implements NetworkService, Configurable {
     private static final Logger s_logger = Logger.getLogger(NetworkServiceImpl.class);
 
+    private static final ConfigKey<Boolean> AllowDuplicateNetworkName = new ConfigKey<Boolean>("Advanced", Boolean.class,
+            "allow.duplicate.networkname", "true", "Allow creating networks with same name in account", true, ConfigKey.Scope.Account);
+    private static final ConfigKey<Boolean> AllowEmptyStartEndIpAddress = new ConfigKey<Boolean>("Advanced", Boolean.class,
+            "allow.empty.start.end.ipaddress", "true", "Allow creating network without mentioning start and end IP address",
+            true, ConfigKey.Scope.Account);
     private static final long MIN_VLAN_ID = 0L;
     private static final long MAX_VLAN_ID = 4095L; // 2^12 - 1
     private static final long MIN_GRE_KEY = 0L;
@@ -313,6 +321,8 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService {
     VpcOfferingDao _vpcOfferingDao;
     @Inject
     AccountService _accountService;
+    @Inject
+    NetworkAccountDao _networkAccountDao;
 
     int _cidrLimit;
     boolean _allowSubdomainNetworkAccess;
@@ -1164,6 +1174,18 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService {
             throw new InvalidParameterValueException("Parameter subDomainAccess can be specified only with aclType=Domain");
         }
 
+        if (aclType == ACLType.Domain) {
+            owner = _accountDao.findById(Account.ACCOUNT_ID_SYSTEM);
+        }
+
+        // The network name is unique under the account
+        if (!AllowDuplicateNetworkName.valueIn(owner.getAccountId())) {
+            List<NetworkVO> existingNetwork = _networksDao.listByAccountIdNetworkName(owner.getId(), name);
+            if (!existingNetwork.isEmpty()) {
+                throw new InvalidParameterValueException("Another network with same name already exists within account: " + owner.getAccountName());
+            }
+        }
+
         boolean ipv4 = true, ipv6 = false;
         if (startIP != null) {
             ipv4 = true;
@@ -1188,6 +1210,15 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService {
             }
         }
 
+        // Start and end IP address are mandatory for shared networks.
+        if (ntwkOff.getGuestType() == GuestType.Shared && vpcId == null) {
+            if (!AllowEmptyStartEndIpAddress.valueIn(owner.getAccountId()) &&
+                (startIP == null && endIP == null) &&
+                (startIPv6 == null && endIPv6 == null)) {
+                throw new InvalidParameterValueException("Either IPv4 or IPv6 start and end address are mandatory");
+            }
+        }
+
         String cidr = null;
         if (ipv4) {
             // if end ip is not specified, default it to startIp
@@ -4537,4 +4568,14 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService {
         return true;
     }
 
+    @Override
+    public String getConfigComponentName() {
+        return NetworkService.class.getSimpleName();
+    }
+
+    @Override
+    public ConfigKey<?>[] getConfigKeys() {
+        return new ConfigKey<?>[] {AllowDuplicateNetworkName, AllowEmptyStartEndIpAddress};
+    }
+
 }

server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java

@@ -235,4 +235,9 @@ public class MockNetworkDaoImpl extends GenericDaoBase<NetworkVO, Long> implemen
     public List<NetworkVO> listNetworkVO(List<Long> idset) {
         return null;
     }
+
+    @Override
+    public List<NetworkVO> listByAccountIdNetworkName(final long accountId, final String name) {
+        return null;
+    }
 }

test/integration/smoke/test_create_network.py

@@ -0,0 +1,291 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.sshClient import SshClient
+from marvin.lib.utils import (cleanup_resources,
+                              random_gen)
+from marvin.lib.base import (Account,
+                             Configurations,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             PhysicalNetwork,
+                             ServiceOffering,
+                             Zone)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_free_vlan)
+import logging
+import random
+
+class TestNetworkManagement(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(
+            TestNetworkManagement,
+            cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+        cls.dbclient = cls.testClient.getDbConnection()
+        cls.testdata = cls.testClient.getParsedTestDataConfig()
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.zone = Zone(zone.__dict__)
+        cls._cleanup = []
+
+        cls.logger = logging.getLogger("TestNetworkManagement")
+        cls.stream_handler = logging.StreamHandler()
+        cls.logger.setLevel(logging.DEBUG)
+        cls.logger.addHandler(cls.stream_handler)
+
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        testClient = super(TestNetworkManagement, cls).getClsTestClient()
+        cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        # Create new domain, account, network and VM
+        cls.user_domain = Domain.create(
+            cls.apiclient,
+            services=cls.testdata["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account
+        cls.account = Account.create(
+            cls.apiclient,
+            cls.testdata["acl"]["accountD2"],
+            admin=True,
+            domainid=cls.user_domain.id
+        )
+
+        # Create small service offering
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.testdata["service_offerings"]["small"]
+        )
+
+        cls._cleanup.append(cls.service_offering)
+        cls._cleanup.append(cls.account)
+        cls._cleanup.append(cls.user_domain)
+
+    @classmethod
+    def tearDownClass(self):
+        try:
+            cleanup_resources(self.apiclient, self._cleanup)
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+        return
+
+    def setUp(self):
+        self.apiclient = self.testClient.getApiClient()
+        self.cleanup = []
+        return
+
+    def tearDown(self):
+        try:
+            cleanup_resources(self.apiclient, self.cleanup)
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+        return
+
+    @attr(tags=["adeancedsg"], required_hardware="false")
+    def test_01_create_network_without_start_end_ip(self):
+        """Create Shared network without start and end ip
+
+            Steps:
+            # 1. Update the global setting allow.empty.start.end.ipaddress to true
+            # 2. Create a shared network without specifying start or end ip
+            # 3. This should create the network
+            # 4. Now Update the global setting allow.empty.start.end.ipaddress to false
+            # 5. Create a shared network without specifying start or end ip
+            # 6. Exception should be thrown since start and end ip are not specified
+        :return:
+        """
+        # Create network offering
+        self.network_offering = NetworkOffering.create(
+            self.apiclient,
+            self.testdata["network_offering_shared"]
+        )
+
+        NetworkOffering.update(
+            self.network_offering,
+            self.apiclient,
+            id=self.network_offering.id,
+            state="enabled"
+        )
+
+        physical_network, vlan = get_free_vlan(self.apiclient, self.zone.id)
+        self.testdata["shared_network_sg"]["physicalnetworkid"] = physical_network.id
+
+        random_subnet_number = random.randrange(100, 199)
+        self.testdata["shared_network_sg"]["specifyVlan"] = 'True'
+        self.testdata["shared_network_sg"]["specifyIpRanges"] = 'True'
+        self.testdata["shared_network_sg"]["name"] = "Shared-Network-SG-Test-vlan" + str(random_subnet_number)
+        self.testdata["shared_network_sg"]["displaytext"] = "Shared-Network-SG-Test-vlan" + str(random_subnet_number)
+        self.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number)
+        self.testdata["shared_network_sg"]["startip"] = None
+        self.testdata["shared_network_sg"]["endip"] = None
+        self.testdata["shared_network_sg"]["gateway"] = "192.168." + str(random_subnet_number) + ".254"
+        self.testdata["shared_network_sg"]["netmask"] = "255.255.255.0"
+        self.testdata["shared_network_sg"]["acltype"] = "account"
+
+        # Update the global setting to true
+        Configurations.update(self.apiclient,
+            name="allow.empty.start.end.ipaddress",
+            value="true"
+        )
+
+        # Create network
+        network = Network.create(
+                self.apiclient,
+                self.testdata["shared_network_sg"],
+                networkofferingid=self.network_offering.id,
+                zoneid=self.zone.id,
+                accountid=self.account.name,
+                domainid=self.account.domainid
+        )
+
+        self.logger.info("network id is %s" % network.id)
+        self.cleanup.append(network)
+
+        # Update the global setting to false
+        Configurations.update(self.apiclient,
+            name="allow.empty.start.end.ipaddress",
+            value="false"
+        )
+
+        # Exception should be thrown
+        with self.assertRaises(Exception):
+            self.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number)
+            network2 = Network.create(
+                self.apiclient,
+                self.testdata["shared_network_sg"],
+                networkofferingid=self.network_offering.id,
+                zoneid=self.zone.id,
+                accountid=self.account.name,
+                domainid=self.account.domainid
+            )
+
+        # Restore the setting to default value which is true
+        Configurations.update(self.apiclient,
+            name="allow.empty.start.end.ipaddress",
+            value="true"
+        )
+
+        self.cleanup.append(self.network_offering)
+
+    @attr(tags=["adeancedsg"], required_hardware="false")
+    def test_02_create_network_with_same_name(self):
+        """Create Shared network with same name in same account
+
+            Steps:
+            # 1. Update the global setting allow.duplicate.networkname to true
+            # 2. Create a shared network in an account
+            # 3. Try to create another shared network with same name in the same account
+            # 4. No exception should be thrown as multiple networks with same name can be created
+            # 5. Now update the global setting allow.duplicate.networkname to false
+            # 6. Try to create another shared network with same name in the same account
+            # 7. Exception should be thrown as network with same name cant be created in the same account
+        :return:
+        """
+        # Update the global setting to true
+        Configurations.update(self.apiclient,
+            name="allow.duplicate.networkname",
+            value="true"
+        )
+
+        # Create network offering
+        self.network_offering = NetworkOffering.create(
+            self.apiclient,
+            self.testdata["network_offering_shared"]
+        )
+
+        NetworkOffering.update(
+            self.network_offering,
+            self.apiclient,
+            id=self.network_offering.id,
+            state="enabled"
+        )
+
+        physical_network, vlan = get_free_vlan(self.apiclient, self.zone.id)
+        self.testdata["shared_network_sg"]["physicalnetworkid"] = physical_network.id
+
+        random_subnet_number = random.randrange(100, 199)
+        self.testdata["shared_network_sg"]["specifyVlan"] = 'True'
+        self.testdata["shared_network_sg"]["specifyIpRanges"] = 'True'
+        self.testdata["shared_network_sg"]["name"] = "Shared-Network-SG-Test-vlan-1"
+        self.testdata["shared_network_sg"]["displaytext"] = "Shared-Network-SG-Test-vlan-1"
+        self.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number)
+        self.testdata["shared_network_sg"]["startip"] = "192.168." + str(random_subnet_number) + ".1"
+        self.testdata["shared_network_sg"]["endip"] = "192.168." + str(random_subnet_number) + ".10"
+        self.testdata["shared_network_sg"]["gateway"] = "192.168." + str(random_subnet_number) + ".254"
+        self.testdata["shared_network_sg"]["netmask"] = "255.255.255.0"
+        self.testdata["shared_network_sg"]["acltype"] = "account"
+
+        # Create the first network
+        network3 = Network.create(
+            self.apiclient,
+            self.testdata["shared_network_sg"],
+            networkofferingid=self.network_offering.id,
+            zoneid=self.zone.id,
+            accountid=self.account.name,
+            domainid=self.account.domainid
+        )
+        self.cleanup.append(network3)
+
+        # Create the second network with same name. No exception should be thrown
+        random_subnet_number = random.randrange(100, 199)
+        self.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number)
+        network4 = Network.create(
+            self.apiclient,
+            self.testdata["shared_network_sg"],
+            networkofferingid=self.network_offering.id,
+            zoneid=self.zone.id,
+            accountid=self.account.name,
+            domainid=self.account.domainid
+        )
+
+        self.cleanup.append(network4)
+
+        # Update the global setting to true
+        Configurations.update(self.apiclient,
+            name="allow.duplicate.networkname",
+            value="false"
+        )
+
+        # Exception should be thrown while creating another network with same name
+        with self.assertRaises(Exception):
+            random_subnet_number = random.randrange(100, 199)
+            self.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number)
+            network5 = Network.create(
+                self.apiclient,
+                self.testdata["shared_network_sg"],
+                networkofferingid=self.network_offering.id,
+                zoneid=self.zone.id,
+                accountid=self.account.name,
+                domainid=self.account.domainid
+            )
+
+        # Update the global setting to original value
+        Configurations.update(self.apiclient,
+            name="allow.duplicate.networkname",
+            value="true"
+        )
+
+        self.cleanup.append(self.network_offering)
+